From 11b13792ba57eef7464540d1fd8ee1aede79cda1 Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben@gnunicorn.org>
Date: Thu, 9 Jan 2020 10:41:10 +0100
Subject: [PATCH 1/6] allow extensions to be extensible from the outside

---
 bin/node/cli/src/service.rs            |  4 +++-
 client/api/src/execution_extensions.rs | 24 ++++++++++++++++++++++--
 client/service/src/builder.rs          |  4 ++++
 client/src/client.rs                   |  2 +-
 4 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs
index 69b05083e862b..c3316f62c370e 100644
--- a/bin/node/cli/src/service.rs
+++ b/bin/node/cli/src/service.rs
@@ -56,7 +56,7 @@ macro_rules! new_full_start {
 		let mut import_setup = None;
 		let inherent_data_providers = sp_inherents::InherentDataProviders::new();
 
-		let builder = sc_service::ServiceBuilder::new_full::<
+		let mut builder = sc_service::ServiceBuilder::new_full::<
 			node_primitives::Block, node_runtime::RuntimeApi, node_executor::Executor
 		>($config)?
 			.with_select_chain(|_config, backend| {
@@ -103,6 +103,8 @@ macro_rules! new_full_start {
 				Ok(node_rpc::create(client, pool, node_rpc::LightDeps::none(fetcher)))
 			})?;
 
+		builder.client().execution_extensions().set_extensions_maker(Box::new(()));
+
 		(builder, import_setup, inherent_data_providers)
 	}}
 }
diff --git a/client/api/src/execution_extensions.rs b/client/api/src/execution_extensions.rs
index 351e9c8914043..20e3f2cc56b41 100644
--- a/client/api/src/execution_extensions.rs
+++ b/client/api/src/execution_extensions.rs
@@ -62,6 +62,18 @@ impl Default for ExecutionStrategies {
 	}
 }
 
+/// Generate the starting set of ExternalitiesExtensions based upon the given capabilities
+pub trait ExtensionsMaker: Send + Sync {
+	/// Make `Extensions` for given Capapbilities
+	fn extensions_for(&self, capabilities: offchain::Capabilities) -> Extensions;
+}
+
+impl ExtensionsMaker for () {
+	fn extensions_for(&self, _capabilities: offchain::Capabilities) -> Extensions {
+		Extensions::new()
+	}
+}
+
 /// A producer of execution extensions for offchain calls.
 ///
 /// This crate aggregates extensions available for the offchain calls
@@ -71,6 +83,7 @@ pub struct ExecutionExtensions<Block: traits::Block> {
 	strategies: ExecutionStrategies,
 	keystore: Option<BareCryptoStorePtr>,
 	transaction_pool: RwLock<Option<Weak<dyn sp_transaction_pool::OffchainSubmitTransaction<Block>>>>,
+	extensions_maker: Box<dyn ExtensionsMaker>,
 }
 
 impl<Block: traits::Block> Default for ExecutionExtensions<Block> {
@@ -79,6 +92,7 @@ impl<Block: traits::Block> Default for ExecutionExtensions<Block> {
 			strategies: Default::default(),
 			keystore: None,
 			transaction_pool: RwLock::new(None),
+			extensions_maker: Box::new(()),
 		}
 	}
 }
@@ -90,7 +104,8 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 		keystore: Option<BareCryptoStorePtr>,
 	) -> Self {
 		let transaction_pool = RwLock::new(None);
-		Self { strategies, keystore, transaction_pool }
+		let extensions_maker = Box::new(());
+		Self { strategies, keystore, extensions_maker, transaction_pool }
 	}
 
 	/// Get a reference to the execution strategies.
@@ -98,6 +113,11 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 		&self.strategies
 	}
 
+	/// Set the new extensions_maker
+	pub fn set_extensions_maker(&mut self, maker: Box<dyn ExtensionsMaker>) {
+		self.extensions_maker = maker;
+	}
+
 	/// Register transaction pool extension.
 	///
 	/// To break retain cycle between `Client` and `TransactionPool` we require this
@@ -135,7 +155,7 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 
 		let capabilities = context.capabilities();
 
-		let mut extensions = Extensions::new();
+		let mut extensions = self.extensions_maker.extensions_for(capabilities);
 
 		if capabilities.has(offchain::Capability::Keystore) {
 			if let Some(keystore) = self.keystore.as_ref() {
diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs
index 8a4d760acc712..a2f7903e4f8aa 100644
--- a/client/service/src/builder.rs
+++ b/client/service/src/builder.rs
@@ -212,6 +212,10 @@ fn new_full_parts<TBl, TRtApi, TExecDisp, TCfg, TGen, TCSExt>(
 			Some(keystore.clone()),
 		);
 
+
+		let backend = Arc::new(sc_client_db::Backend::new(db_config, CANONICALIZATION_DELAY)?);
+		let executor = sc_client::LocalCallExecutor::new(backend.clone(), executor);
+
 		sc_client_db::new_client(
 			db_config,
 			executor,
diff --git a/client/src/client.rs b/client/src/client.rs
index f99789de7da97..8d2ea5cd77890 100644
--- a/client/src/client.rs
+++ b/client/src/client.rs
@@ -243,7 +243,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 	/// Get a reference to the execution extensions.
 	pub fn execution_extensions(&self) -> &ExecutionExtensions<Block> {
 		&self.execution_extensions
-	}
+    }
 
 	/// Get a reference to the state at a given block.
 	pub fn state_at(&self, block: &BlockId<Block>) -> sp_blockchain::Result<B::State> {

From 6a37005fd6b82b42453d5d82b94ddbcffe5ed058 Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben@gnunicorn.org>
Date: Thu, 9 Jan 2020 17:11:21 +0100
Subject: [PATCH 2/6] expose ExecutionExtensionsMaker to the public

---
 bin/node/cli/src/service.rs            |  2 +-
 client/api/src/execution_extensions.rs | 12 +++----
 client/service/src/builder.rs          | 45 +++++++++++++++++++++++---
 client/src/client.rs                   |  2 +-
 4 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs
index c3316f62c370e..3abe2ab711b52 100644
--- a/bin/node/cli/src/service.rs
+++ b/bin/node/cli/src/service.rs
@@ -56,7 +56,7 @@ macro_rules! new_full_start {
 		let mut import_setup = None;
 		let inherent_data_providers = sp_inherents::InherentDataProviders::new();
 
-		let mut builder = sc_service::ServiceBuilder::new_full::<
+		let builder = sc_service::ServiceBuilder::new_full::<
 			node_primitives::Block, node_runtime::RuntimeApi, node_executor::Executor
 		>($config)?
 			.with_select_chain(|_config, backend| {
diff --git a/client/api/src/execution_extensions.rs b/client/api/src/execution_extensions.rs
index 20e3f2cc56b41..7936e0e938313 100644
--- a/client/api/src/execution_extensions.rs
+++ b/client/api/src/execution_extensions.rs
@@ -83,7 +83,7 @@ pub struct ExecutionExtensions<Block: traits::Block> {
 	strategies: ExecutionStrategies,
 	keystore: Option<BareCryptoStorePtr>,
 	transaction_pool: RwLock<Option<Weak<dyn sp_transaction_pool::OffchainSubmitTransaction<Block>>>>,
-	extensions_maker: Box<dyn ExtensionsMaker>,
+	extensions_maker: RwLock<Box<dyn ExtensionsMaker>>,
 }
 
 impl<Block: traits::Block> Default for ExecutionExtensions<Block> {
@@ -92,7 +92,7 @@ impl<Block: traits::Block> Default for ExecutionExtensions<Block> {
 			strategies: Default::default(),
 			keystore: None,
 			transaction_pool: RwLock::new(None),
-			extensions_maker: Box::new(()),
+			extensions_maker: RwLock::new(Box::new(())),
 		}
 	}
 }
@@ -105,7 +105,7 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 	) -> Self {
 		let transaction_pool = RwLock::new(None);
 		let extensions_maker = Box::new(());
-		Self { strategies, keystore, extensions_maker, transaction_pool }
+		Self { strategies, keystore, extensions_maker: RwLock::new(extensions_maker), transaction_pool }
 	}
 
 	/// Get a reference to the execution strategies.
@@ -114,8 +114,8 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 	}
 
 	/// Set the new extensions_maker
-	pub fn set_extensions_maker(&mut self, maker: Box<dyn ExtensionsMaker>) {
-		self.extensions_maker = maker;
+	pub fn set_extensions_maker(&self, maker: Box<dyn ExtensionsMaker>) {
+		*self.extensions_maker.write() = maker;
 	}
 
 	/// Register transaction pool extension.
@@ -155,7 +155,7 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 
 		let capabilities = context.capabilities();
 
-		let mut extensions = self.extensions_maker.extensions_for(capabilities);
+		let mut extensions = self.extensions_maker.read().extensions_for(capabilities);
 
 		if capabilities.has(offchain::Capability::Keystore) {
 			if let Some(keystore) = self.keystore.as_ref() {
diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs
index a2f7903e4f8aa..e6fc587242cb1 100644
--- a/client/service/src/builder.rs
+++ b/client/service/src/builder.rs
@@ -22,6 +22,7 @@ use sc_client_api::{
 	self,
 	BlockchainEvents,
 	backend::RemoteBackend, light::RemoteBlockchain,
+	execution_extensions::ExtensionsMaker,
 };
 use sc_client::Client;
 use sc_chain_spec::{RuntimeGenesis, Extension};
@@ -212,10 +213,6 @@ fn new_full_parts<TBl, TRtApi, TExecDisp, TCfg, TGen, TCSExt>(
 			Some(keystore.clone()),
 		);
 
-
-		let backend = Arc::new(sc_client_db::Backend::new(db_config, CANONICALIZATION_DELAY)?);
-		let executor = sc_client::LocalCallExecutor::new(backend.clone(), executor);
-
 		sc_client_db::new_client(
 			db_config,
 			executor,
@@ -744,6 +741,46 @@ ServiceBuilder<
 		+ TransactionPoolMaintainer<Block=TBl, Hash = <TBl as BlockT>::Hash>,
 	TRpc: sc_rpc::RpcExtension<sc_rpc::Metadata> + Clone,
 {
+
+	/// Set an ExecutionExtensionsMaker
+	pub fn with_execution_extensions_maker(self, execution_extensions_maker: Box<dyn ExtensionsMaker>) -> Result<Self, Error> {
+		let ServiceBuilder {
+			config,
+			client,
+			backend,
+			keystore,
+			fetcher,
+			select_chain,
+			import_queue,
+			finality_proof_request_builder,
+			finality_proof_provider,
+			network_protocol,
+			transaction_pool,
+			rpc_extensions,
+			remote_backend,
+			marker
+		} = self;
+
+		client.execution_extensions().set_extensions_maker(execution_extensions_maker);
+
+		Ok(ServiceBuilder {
+			config,
+			client,
+			backend,
+			keystore,
+			fetcher,
+			select_chain,
+			import_queue,
+			finality_proof_request_builder,
+			finality_proof_provider,
+			network_protocol,
+			transaction_pool,
+			rpc_extensions,
+			remote_backend,
+			marker,
+		})
+	}
+
 	/// Builds the service.
 	pub fn build(self) -> Result<Service<
 		TBl,
diff --git a/client/src/client.rs b/client/src/client.rs
index 8d2ea5cd77890..f99789de7da97 100644
--- a/client/src/client.rs
+++ b/client/src/client.rs
@@ -243,7 +243,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 	/// Get a reference to the execution extensions.
 	pub fn execution_extensions(&self) -> &ExecutionExtensions<Block> {
 		&self.execution_extensions
-    }
+	}
 
 	/// Get a reference to the state at a given block.
 	pub fn state_at(&self, block: &BlockId<Block>) -> sp_blockchain::Result<B::State> {

From 5969adc19d84fba304ffddfda047bf15be1b5c80 Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben@gnunicorn.org>
Date: Thu, 9 Jan 2020 17:16:59 +0100
Subject: [PATCH 3/6] drop unnecessary access point

---
 bin/node/cli/src/service.rs | 2 --
 1 file changed, 2 deletions(-)

diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs
index 3abe2ab711b52..69b05083e862b 100644
--- a/bin/node/cli/src/service.rs
+++ b/bin/node/cli/src/service.rs
@@ -103,8 +103,6 @@ macro_rules! new_full_start {
 				Ok(node_rpc::create(client, pool, node_rpc::LightDeps::none(fetcher)))
 			})?;
 
-		builder.client().execution_extensions().set_extensions_maker(Box::new(()));
-
 		(builder, import_setup, inherent_data_providers)
 	}}
 }

From cae0ed52cf7877e0c165f74b9846125b4877a606 Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben@gnunicorn.org>
Date: Fri, 10 Jan 2020 10:37:22 +0100
Subject: [PATCH 4/6] Naming things

---
 client/api/src/execution_extensions.rs | 20 ++++++------
 client/service/src/builder.rs          | 43 +++-----------------------
 2 files changed, 15 insertions(+), 48 deletions(-)

diff --git a/client/api/src/execution_extensions.rs b/client/api/src/execution_extensions.rs
index 7936e0e938313..847a26ce1a1a5 100644
--- a/client/api/src/execution_extensions.rs
+++ b/client/api/src/execution_extensions.rs
@@ -63,12 +63,12 @@ impl Default for ExecutionStrategies {
 }
 
 /// Generate the starting set of ExternalitiesExtensions based upon the given capabilities
-pub trait ExtensionsMaker: Send + Sync {
+pub trait ExtensionsFactory: Send + Sync {
 	/// Make `Extensions` for given Capapbilities
 	fn extensions_for(&self, capabilities: offchain::Capabilities) -> Extensions;
 }
 
-impl ExtensionsMaker for () {
+impl ExtensionsFactory for () {
 	fn extensions_for(&self, _capabilities: offchain::Capabilities) -> Extensions {
 		Extensions::new()
 	}
@@ -83,7 +83,7 @@ pub struct ExecutionExtensions<Block: traits::Block> {
 	strategies: ExecutionStrategies,
 	keystore: Option<BareCryptoStorePtr>,
 	transaction_pool: RwLock<Option<Weak<dyn sp_transaction_pool::OffchainSubmitTransaction<Block>>>>,
-	extensions_maker: RwLock<Box<dyn ExtensionsMaker>>,
+	extensions_factory: RwLock<Box<dyn ExtensionsFactory>>,
 }
 
 impl<Block: traits::Block> Default for ExecutionExtensions<Block> {
@@ -92,7 +92,7 @@ impl<Block: traits::Block> Default for ExecutionExtensions<Block> {
 			strategies: Default::default(),
 			keystore: None,
 			transaction_pool: RwLock::new(None),
-			extensions_maker: RwLock::new(Box::new(())),
+			extensions_factory: RwLock::new(Box::new(())),
 		}
 	}
 }
@@ -104,8 +104,8 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 		keystore: Option<BareCryptoStorePtr>,
 	) -> Self {
 		let transaction_pool = RwLock::new(None);
-		let extensions_maker = Box::new(());
-		Self { strategies, keystore, extensions_maker: RwLock::new(extensions_maker), transaction_pool }
+		let extensions_factory = Box::new(());
+		Self { strategies, keystore, extensions_factory: RwLock::new(extensions_factory), transaction_pool }
 	}
 
 	/// Get a reference to the execution strategies.
@@ -113,9 +113,9 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 		&self.strategies
 	}
 
-	/// Set the new extensions_maker
-	pub fn set_extensions_maker(&self, maker: Box<dyn ExtensionsMaker>) {
-		*self.extensions_maker.write() = maker;
+	/// Set the new extensions_factory
+	pub fn set_extensions_factory(&self, maker: Box<dyn ExtensionsFactory>) {
+		*self.extensions_factory.write() = maker;
 	}
 
 	/// Register transaction pool extension.
@@ -155,7 +155,7 @@ impl<Block: traits::Block> ExecutionExtensions<Block> {
 
 		let capabilities = context.capabilities();
 
-		let mut extensions = self.extensions_maker.read().extensions_for(capabilities);
+		let mut extensions = self.extensions_factory.read().extensions_for(capabilities);
 
 		if capabilities.has(offchain::Capability::Keystore) {
 			if let Some(keystore) = self.keystore.as_ref() {
diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs
index e6fc587242cb1..f0b132c87099d 100644
--- a/client/service/src/builder.rs
+++ b/client/service/src/builder.rs
@@ -22,7 +22,7 @@ use sc_client_api::{
 	self,
 	BlockchainEvents,
 	backend::RemoteBackend, light::RemoteBlockchain,
-	execution_extensions::ExtensionsMaker,
+	execution_extensions::ExtensionsFactory,
 };
 use sc_client::Client;
 use sc_chain_spec::{RuntimeGenesis, Extension};
@@ -742,43 +742,10 @@ ServiceBuilder<
 	TRpc: sc_rpc::RpcExtension<sc_rpc::Metadata> + Clone,
 {
 
-	/// Set an ExecutionExtensionsMaker
-	pub fn with_execution_extensions_maker(self, execution_extensions_maker: Box<dyn ExtensionsMaker>) -> Result<Self, Error> {
-		let ServiceBuilder {
-			config,
-			client,
-			backend,
-			keystore,
-			fetcher,
-			select_chain,
-			import_queue,
-			finality_proof_request_builder,
-			finality_proof_provider,
-			network_protocol,
-			transaction_pool,
-			rpc_extensions,
-			remote_backend,
-			marker
-		} = self;
-
-		client.execution_extensions().set_extensions_maker(execution_extensions_maker);
-
-		Ok(ServiceBuilder {
-			config,
-			client,
-			backend,
-			keystore,
-			fetcher,
-			select_chain,
-			import_queue,
-			finality_proof_request_builder,
-			finality_proof_provider,
-			network_protocol,
-			transaction_pool,
-			rpc_extensions,
-			remote_backend,
-			marker,
-		})
+	/// Set an ExecutionExtensionsFactory
+	pub fn with_execution_extensions_factory(self, execution_extensions_factory: Box<dyn ExtensionsFactory>) -> Result<Self, Error> {
+		self.client.execution_extensions().set_extensions_factory(execution_extensions_factory);
+		Ok(self)
 	}
 
 	/// Builds the service.

From 295b60f479901ee7bd2af0ab7f9b4a2d3b98322c Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben@gnunicorn.org>
Date: Fri, 10 Jan 2020 10:51:17 +0100
Subject: [PATCH 5/6] Add linked FIXME

---
 client/api/src/execution_extensions.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/client/api/src/execution_extensions.rs b/client/api/src/execution_extensions.rs
index 847a26ce1a1a5..fcc67bb61c1b9 100644
--- a/client/api/src/execution_extensions.rs
+++ b/client/api/src/execution_extensions.rs
@@ -82,6 +82,8 @@ impl ExtensionsFactory for () {
 pub struct ExecutionExtensions<Block: traits::Block> {
 	strategies: ExecutionStrategies,
 	keystore: Option<BareCryptoStorePtr>,
+	// FIXME: these two are only RwLock because of https://github.com/paritytech/substrate/issues/4587
+	//        remove when fixed.
 	transaction_pool: RwLock<Option<Weak<dyn sp_transaction_pool::OffchainSubmitTransaction<Block>>>>,
 	extensions_factory: RwLock<Box<dyn ExtensionsFactory>>,
 }
@@ -95,7 +97,7 @@ impl<Block: traits::Block> Default for ExecutionExtensions<Block> {
 			extensions_factory: RwLock::new(Box::new(())),
 		}
 	}
-}
+}https://github.com/paritytech/substrate/issues/4587
 
 impl<Block: traits::Block> ExecutionExtensions<Block> {
 	/// Create new `ExecutionExtensions` given a `keystore` and `ExecutionStrategies`.

From b381fb437397beeca208c7417b0514973246061d Mon Sep 17 00:00:00 2001
From: Benjamin Kampmann <ben@gnunicorn.org>
Date: Fri, 10 Jan 2020 10:52:59 +0100
Subject: [PATCH 6/6] grmmml

---
 client/api/src/execution_extensions.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/client/api/src/execution_extensions.rs b/client/api/src/execution_extensions.rs
index fcc67bb61c1b9..1a986a23a47dc 100644
--- a/client/api/src/execution_extensions.rs
+++ b/client/api/src/execution_extensions.rs
@@ -97,7 +97,7 @@ impl<Block: traits::Block> Default for ExecutionExtensions<Block> {
 			extensions_factory: RwLock::new(Box::new(())),
 		}
 	}
-}https://github.com/paritytech/substrate/issues/4587
+}
 
 impl<Block: traits::Block> ExecutionExtensions<Block> {
 	/// Create new `ExecutionExtensions` given a `keystore` and `ExecutionStrategies`.