Add documentation to SubmitSignedTransaction and actually make it work (#4200)

* Add documentation to signed transactions and actually make them work.

* Fix naming and bounds.

* Forgotten import.

* Remove warning.

* Make accounts optional, fix logic.

* Split the method to avoid confusing type error message.

* Move executor tests to integration.

* Add submit transactions tests.

* Make `submit_transaction` tests compile

* Remove a file that was accidently committed

* Add can_sign helper function.

* Fix compilation.

* Add a key to keystore.

* Fix the tests.

* Remove env_logger.

* Fix sending multiple transactions.

* Remove commented code.

* Bring back criterion.

* Remove stray debug log.

* Apply suggestions from code review

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Make sure to initialize block correctly.

* Initialize block for offchain workers.

* Add test for transaction validity.

* Fix tests.

* Review suggestions.

* Remove redundant comment.

* Make sure to use correct block number of authoring.

* Change the runtime API.

* Support both versions.

* Bump spec version, fix RPC test.

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>
Co-authored-by: Gavin Wood <github@gavwood.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

substrate/client/offchain/src/lib.rs

@@ -42,7 +42,7 @@ use futures::future::Future;
 use log::{debug, warn};
 use sc_network::NetworkStateInfo;
 use sp_core::{offchain::{self, OffchainStorage}, ExecutionContext};
-use sp_runtime::{generic::BlockId, traits::{self, ProvideRuntimeApi}};
+use sp_runtime::{generic::BlockId, traits::{self, ProvideRuntimeApi, Header}};
 
 mod api;
 
@@ -92,33 +92,47 @@ impl<Client, Storage, Block> OffchainWorkers<
 	#[must_use]
 	pub fn on_block_imported(
 		&self,
-		number: &<Block::Header as traits::Header>::Number,
+		header: &Block::Header,
 		network_state: Arc<dyn NetworkStateInfo + Send + Sync>,
 		is_validator: bool,
 	) -> impl Future<Output = ()> {
 		let runtime = self.client.runtime_api();
-		let at = BlockId::number(*number);
-		let has_api = runtime.has_api::<dyn OffchainWorkerApi<Block, Error = ()>>(&at);
-		debug!("Checking offchain workers at {:?}: {:?}", at, has_api);
+		let at = BlockId::number(*header.number());
+		let has_api_v1 = runtime.has_api_with::<dyn OffchainWorkerApi<Block, Error = ()>, _>(
+			&at, |v| v == 1
+		);
+		let has_api_v2 = runtime.has_api::<dyn OffchainWorkerApi<Block, Error = ()>>(&at);
+		let version = match (has_api_v1, has_api_v2) {
+			(_, Ok(true)) => 2,
+			(Ok(true), _) => 1,
+			_ => 0,
+		};
 
-		if has_api.unwrap_or(false) {
+		debug!("Checking offchain workers at {:?}: version:{}", at, version);
+		if version > 0 {
 			let (api, runner) = api::AsyncApi::new(
 				self.db.clone(),
 				network_state.clone(),
 				is_validator,
 			);
 			debug!("Spawning offchain workers at {:?}", at);
-			let number = *number;
+			let header = header.clone();
 			let client = self.client.clone();
 			self.spawn_worker(move || {
 				let runtime = client.runtime_api();
 				let api = Box::new(api);
 				debug!("Running offchain workers at {:?}", at);
-				let run = runtime.offchain_worker_with_context(
-					&at,
-					ExecutionContext::OffchainCall(Some((api, offchain::Capabilities::all()))),
-					number,
-				);
+				let context = ExecutionContext::OffchainCall(Some(
+					(api, offchain::Capabilities::all())
+				));
+				let run = if version == 2 {
+					runtime.offchain_worker_with_context(&at, context, &header)
+				} else {
+					#[allow(deprecated)]
+					runtime.offchain_worker_before_version_2_with_context(
+						&at, context, *header.number()
+					)
+				};
 				if let Err(e) =	run {
 					log::error!("Error running offchain workers at {:?}: {:?}", at, e);
 				}
@@ -150,6 +164,7 @@ mod tests {
 	use substrate_test_runtime_client::runtime::Block;
 	use sc_transaction_pool::{BasicPool, FullChainApi};
 	use sp_transaction_pool::{TransactionPool, InPoolTransaction};
+	use sp_runtime::{generic::Header, traits::Header as _};
 
 	struct MockNetworkStateInfo();
 
@@ -169,7 +184,7 @@ mod tests {
 		fn submit_at(
 			&self,
 			at: &BlockId<Block>,
-			extrinsic: <Block as sp_runtime::traits::Block>::Extrinsic,
+			extrinsic: <Block as traits::Block>::Extrinsic,
 		) -> Result<(), ()> {
 			futures::executor::block_on(self.0.submit_one(&at, extrinsic))
 				.map(|_| ())
@@ -187,10 +202,17 @@ mod tests {
 			.register_transaction_pool(Arc::downgrade(&pool.clone()) as _);
 		let db = sc_client_db::offchain::LocalStorage::new_test();
 		let network_state = Arc::new(MockNetworkStateInfo());
+		let header = Header::new(
+			0u64,
+			Default::default(),
+			Default::default(),
+			Default::default(),
+			Default::default(),
+		);
 
 		// when
 		let offchain = OffchainWorkers::new(client, db);
-		futures::executor::block_on(offchain.on_block_imported(&0u64, network_state, false));
+		futures::executor::block_on(offchain.on_block_imported(&header, network_state, false));
 
 		// then
 		assert_eq!(pool.0.status().ready, 1);

substrate/client/rpc/src/state/tests.rs

@@ -400,7 +400,7 @@ fn should_return_runtime_version() {
 		\"specVersion\":1,\"implVersion\":1,\"apis\":[[\"0xdf6acb689907609b\",2],\
 		[\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",1],[\"0x40fe3ad401f8959a\",4],\
 		[\"0xc6e9a76309f39b09\",1],[\"0xdd718d5cc53262d4\",1],[\"0xcbca25e39f142387\",1],\
-		[\"0xf78b278be53f454c\",1],[\"0xab3c0572291feb8b\",1],[\"0xbc9d89904f5b923f\",1]]}";
+		[\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],[\"0xbc9d89904f5b923f\",1]]}";
 
 	let runtime_version = api.runtime_version(None.into()).wait().unwrap();
 	let serialized = serde_json::to_string(&runtime_version).unwrap();

substrate/client/service/src/builder.rs

@@ -43,7 +43,7 @@ use sc_rpc;
 use sp_api::ConstructRuntimeApi;
 use sp_runtime::generic::BlockId;
 use sp_runtime::traits::{
-	Block as BlockT, ProvideRuntimeApi, NumberFor, Header, SaturatedConversion,
+	Block as BlockT, ProvideRuntimeApi, NumberFor, SaturatedConversion,
 };
 use sc_executor::{NativeExecutor, NativeExecutionDispatch};
 use std::{
@@ -867,7 +867,6 @@ ServiceBuilder<
 			let events = client.import_notification_stream()
 				.map(|v| Ok::<_, ()>(v)).compat()
 				.for_each(move |notification| {
-					let number = *notification.header.number();
 					let txpool = txpool.upgrade();
 
 					if let Some(txpool) = txpool.as_ref() {
@@ -880,8 +879,11 @@ ServiceBuilder<
 
 					let offchain = offchain.as_ref().and_then(|o| o.upgrade());
 					if let Some(offchain) = offchain {
-						let future = offchain.on_block_imported(&number, network_state_info.clone(), is_validator)
-							.map(|()| Ok(()));
+						let future = offchain.on_block_imported(
+							&notification.header,
+							network_state_info.clone(),
+							is_validator
+						).map(|()| Ok(()));
 						let _ = to_spawn_tx_.unbounded_send(Box::new(Compat::new(future)));
 					}