rpc: bump jsonrpsee v0.22 and fix race in rpc v2 chain_head (#3230)

Close #2992 

Breaking changes:
- rpc server grafana metric `substrate_rpc_requests_started` is removed
(not possible to implement anymore)
- rpc server grafana metric `substrate_rpc_requests_finished` is removed
(not possible to implement anymore)
- rpc server ws ping/pong not ACK:ed within 30 seconds more than three
times then the connection will be closed

Added
- rpc server grafana metric `substrate_rpc_sessions_time` is added to
get the duration for each websocket session

substrate/client/rpc-spec-v2/src/archive/tests.rs

@@ -32,8 +32,7 @@ use super::{
 use assert_matches::assert_matches;
 use codec::{Decode, Encode};
 use jsonrpsee::{
-	core::{EmptyServerParams as EmptyParams, Error},
-	rpc_params, RpcModule,
+	core::EmptyServerParams as EmptyParams, rpc_params, MethodsError as Error, RpcModule,
 };
 use sc_block_builder::BlockBuilderBuilder;
 use sc_client_api::ChildInfo;
@@ -294,7 +293,7 @@ async fn archive_call() {
 		)
 		.await
 		.unwrap_err();
-	assert_matches!(err, Error::Call(err) if err.code() == 3001 && err.message().contains("Invalid parameter"));
+	assert_matches!(err, Error::JsonRpc(err) if err.code() == 3001 && err.message().contains("Invalid parameter"));
 
 	// Pass an invalid parameters that cannot be decode.
 	let err = api
@@ -305,7 +304,7 @@ async fn archive_call() {
 		)
 		.await
 		.unwrap_err();
-	assert_matches!(err, Error::Call(err) if err.code() == 3001 && err.message().contains("Invalid parameter"));
+	assert_matches!(err, Error::JsonRpc(err) if err.code() == 3001 && err.message().contains("Invalid parameter"));
 
 	// Invalid hash.
 	let result: MethodResult = api

substrate/client/rpc-spec-v2/src/chain_head/api.rs

@@ -26,7 +26,7 @@ use crate::{
 	},
 	common::events::StorageQuery,
 };
-use jsonrpsee::proc_macros::rpc;
+use jsonrpsee::{proc_macros::rpc, server::ResponsePayload};
 use sp_rpc::list::ListOrValue;
 
 #[rpc(client, server)]
@@ -59,7 +59,7 @@ pub trait ChainHeadApi<Hash> {
 		&self,
 		follow_subscription: String,
 		hash: Hash,
-	) -> Result<MethodResponse, Error>;
+	) -> ResponsePayload<'static, MethodResponse>;
 
 	/// Retrieves the header of a pinned block.
 	///
@@ -92,7 +92,7 @@ pub trait ChainHeadApi<Hash> {
 		hash: Hash,
 		items: Vec<StorageQuery<String>>,
 		child_trie: Option<String>,
-	) -> Result<MethodResponse, Error>;
+	) -> ResponsePayload<'static, MethodResponse>;
 
 	/// Call into the Runtime API at a specified block's state.
 	///
@@ -106,7 +106,7 @@ pub trait ChainHeadApi<Hash> {
 		hash: Hash,
 		function: String,
 		call_parameters: String,
-	) -> Result<MethodResponse, Error>;
+	) -> ResponsePayload<'static, MethodResponse>;
 
 	/// Unpin a block or multiple blocks reported by the `follow` method.
 	///

substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs

@@ -36,7 +36,8 @@ use crate::{
 use codec::Encode;
 use futures::future::FutureExt;
 use jsonrpsee::{
-	core::async_trait, types::SubscriptionId, PendingSubscriptionSink, SubscriptionSink,
+	core::async_trait, server::ResponsePayload, types::SubscriptionId, MethodResponseFuture,
+	PendingSubscriptionSink, SubscriptionSink,
 };
 use log::debug;
 use sc_client_api::{
@@ -218,16 +219,17 @@ where
 		&self,
 		follow_subscription: String,
 		hash: Block::Hash,
-	) -> Result<MethodResponse, ChainHeadRpcError> {
+	) -> ResponsePayload<'static, MethodResponse> {
 		let mut block_guard = match self.subscriptions.lock_block(&follow_subscription, hash, 1) {
 			Ok(block) => block,
 			Err(SubscriptionManagementError::SubscriptionAbsent) |
-			Err(SubscriptionManagementError::ExceededLimits) => return Ok(MethodResponse::LimitReached),
+			Err(SubscriptionManagementError::ExceededLimits) =>
+				return ResponsePayload::success(MethodResponse::LimitReached),
 			Err(SubscriptionManagementError::BlockHashAbsent) => {
 				// Block is not part of the subscription.
-				return Err(ChainHeadRpcError::InvalidBlock.into())
+				return ResponsePayload::error(ChainHeadRpcError::InvalidBlock);
 			},
-			Err(_) => return Err(ChainHeadRpcError::InvalidBlock.into()),
+			Err(_) => return ResponsePayload::error(ChainHeadRpcError::InvalidBlock),
 		};
 
 		let operation_id = block_guard.operation().operation_id();
@@ -254,7 +256,7 @@ where
 					hash
 				);
 				self.subscriptions.remove_subscription(&follow_subscription);
-				return Err(ChainHeadRpcError::InvalidBlock.into())
+				return ResponsePayload::error(ChainHeadRpcError::InvalidBlock)
 			},
 			Err(error) => FollowEvent::<Block::Hash>::OperationError(OperationError {
 				operation_id: operation_id.clone(),
@@ -262,8 +264,20 @@ where
 			}),
 		};
 
-		let _ = block_guard.response_sender().unbounded_send(event);
-		Ok(MethodResponse::Started(MethodResponseStarted { operation_id, discarded_items: None }))
+		let (rp, rp_fut) = method_started_response(operation_id, None);
+
+		let fut = async move {
+			// Events should only by generated
+			// if the response was successfully propagated.
+			if rp_fut.await.is_err() {
+				return;
+			}
+			let _ = block_guard.response_sender().unbounded_send(event);
+		};
+
+		self.executor.spawn("substrate-rpc-subscription", Some("rpc"), fut.boxed());
+
+		rp
 	}
 
 	fn chain_head_unstable_header(
@@ -294,31 +308,40 @@ where
 		hash: Block::Hash,
 		items: Vec<StorageQuery<String>>,
 		child_trie: Option<String>,
-	) -> Result<MethodResponse, ChainHeadRpcError> {
+	) -> ResponsePayload<'static, MethodResponse> {
 		// Gain control over parameter parsing and returned error.
-		let items = items
+		let items = match items
 			.into_iter()
 			.map(|query| {
 				let key = StorageKey(parse_hex_param(query.key)?);
 				Ok(StorageQuery { key, query_type: query.query_type })
 			})
-			.collect::<Result<Vec<_>, ChainHeadRpcError>>()?;
+			.collect::<Result<Vec<_>, ChainHeadRpcError>>()
+		{
+			Ok(items) => items,
+			Err(err) => {
+				return ResponsePayload::error(err);
+			},
+		};
 
-		let child_trie = child_trie
-			.map(|child_trie| parse_hex_param(child_trie))
-			.transpose()?
-			.map(ChildInfo::new_default_from_vec);
+		let child_trie = match child_trie.map(|child_trie| parse_hex_param(child_trie)).transpose()
+		{
+			Ok(c) => c.map(ChildInfo::new_default_from_vec),
+			Err(e) => return ResponsePayload::error(e),
+		};
 
 		let mut block_guard =
 			match self.subscriptions.lock_block(&follow_subscription, hash, items.len()) {
 				Ok(block) => block,
 				Err(SubscriptionManagementError::SubscriptionAbsent) |
-				Err(SubscriptionManagementError::ExceededLimits) => return Ok(MethodResponse::LimitReached),
+				Err(SubscriptionManagementError::ExceededLimits) => {
+					return ResponsePayload::success(MethodResponse::LimitReached);
+				},
 				Err(SubscriptionManagementError::BlockHashAbsent) => {
 					// Block is not part of the subscription.
-					return Err(ChainHeadRpcError::InvalidBlock.into())
+					return ResponsePayload::error(ChainHeadRpcError::InvalidBlock)
 				},
-				Err(_) => return Err(ChainHeadRpcError::InvalidBlock.into()),
+				Err(_) => return ResponsePayload::error(ChainHeadRpcError::InvalidBlock),
 			};
 
 		let mut storage_client = ChainHeadStorage::<Client, Block, BE>::new(
@@ -334,16 +357,21 @@ where
 		let mut items = items;
 		items.truncate(num_operations);
 
+		let (rp, rp_is_success) = method_started_response(operation_id, Some(discarded));
+
 		let fut = async move {
+			// Events should only by generated
+			// if the response was successfully propagated.
+			if rp_is_success.await.is_err() {
+				return;
+			}
 			storage_client.generate_events(block_guard, hash, items, child_trie).await;
 		};
 
 		self.executor
 			.spawn_blocking("substrate-rpc-subscription", Some("rpc"), fut.boxed());
-		Ok(MethodResponse::Started(MethodResponseStarted {
-			operation_id,
-			discarded_items: Some(discarded),
-		}))
+
+		rp
 	}
 
 	fn chain_head_unstable_call(
@@ -352,29 +380,31 @@ where
 		hash: Block::Hash,
 		function: String,
 		call_parameters: String,
-	) -> Result<MethodResponse, ChainHeadRpcError> {
-		let call_parameters = Bytes::from(parse_hex_param(call_parameters)?);
+	) -> ResponsePayload<'static, MethodResponse> {
+		let call_parameters = match parse_hex_param(call_parameters) {
+			Ok(hex) => Bytes::from(hex),
+			Err(err) => return ResponsePayload::error(err),
+		};
 
 		let mut block_guard = match self.subscriptions.lock_block(&follow_subscription, hash, 1) {
 			Ok(block) => block,
 			Err(SubscriptionManagementError::SubscriptionAbsent) |
 			Err(SubscriptionManagementError::ExceededLimits) => {
 				// Invalid invalid subscription ID.
-				return Ok(MethodResponse::LimitReached)
+				return ResponsePayload::success(MethodResponse::LimitReached)
 			},
 			Err(SubscriptionManagementError::BlockHashAbsent) => {
 				// Block is not part of the subscription.
-				return Err(ChainHeadRpcError::InvalidBlock.into())
+				return ResponsePayload::error(ChainHeadRpcError::InvalidBlock)
 			},
-			Err(_) => return Err(ChainHeadRpcError::InvalidBlock.into()),
+			Err(_) => return ResponsePayload::error(ChainHeadRpcError::InvalidBlock),
 		};
 
 		// Reject subscription if with_runtime is false.
 		if !block_guard.has_runtime() {
-			return Err(ChainHeadRpcError::InvalidRuntimeCall(
+			return ResponsePayload::error(ChainHeadRpcError::InvalidRuntimeCall(
 				"The runtime updates flag must be set".to_string(),
-			)
-			.into())
+			));
 		}
 
 		let operation_id = block_guard.operation().operation_id();
@@ -395,11 +425,20 @@ where
 				})
 			});
 
-		let _ = block_guard.response_sender().unbounded_send(event);
-		Ok(MethodResponse::Started(MethodResponseStarted {
-			operation_id: operation_id.clone(),
-			discarded_items: None,
-		}))
+		let (rp, rp_fut) = method_started_response(operation_id, None);
+
+		let fut = async move {
+			// Events should only by generated
+			// if the response was successfully propagated.
+			if rp_fut.await.is_err() {
+				return;
+			}
+			let _ = block_guard.response_sender().unbounded_send(event);
+		};
+
+		self.executor.spawn("substrate-rpc-subscription", Some("rpc"), fut.boxed());
+
+		rp
 	}
 
 	fn chain_head_unstable_unpin(
@@ -463,3 +502,11 @@ where
 		Ok(())
 	}
 }
+
+fn method_started_response(
+	operation_id: String,
+	discarded_items: Option<usize>,
+) -> (ResponsePayload<'static, MethodResponse>, MethodResponseFuture) {
+	let rp = MethodResponse::Started(MethodResponseStarted { operation_id, discarded_items });
+	ResponsePayload::success(rp).notify_on_completion()
+}

substrate/client/rpc-spec-v2/src/chain_head/tests.rs

@@ -27,8 +27,7 @@ use assert_matches::assert_matches;
 use codec::{Decode, Encode};
 use futures::Future;
 use jsonrpsee::{
-	core::{error::Error, server::Subscription as RpcSubscription},
-	rpc_params, RpcModule,
+	core::server::Subscription as RpcSubscription, rpc_params, MethodsError as Error, RpcModule,
 };
 use sc_block_builder::BlockBuilderBuilder;
 use sc_client_api::ChildInfo;
@@ -359,7 +358,7 @@ async fn get_header() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
+		Error::JsonRpc(ref err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Obtain the valid header.
@@ -388,7 +387,7 @@ async fn get_body() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
+		Error::JsonRpc(ref err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Valid call.
@@ -473,7 +472,7 @@ async fn call_runtime() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
+		Error::JsonRpc(ref err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Pass an invalid parameters that cannot be decode.
@@ -486,7 +485,7 @@ async fn call_runtime() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::json_rpc_spec::INVALID_PARAM_ERROR && err.message().contains("Invalid parameter")
+		Error::JsonRpc(err) if err.code() == super::error::json_rpc_spec::INVALID_PARAM_ERROR && err.message().contains("Invalid parameter")
 	);
 
 	// Valid call.
@@ -589,7 +588,7 @@ async fn call_runtime_without_flag() {
 		.unwrap_err();
 
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_RUNTIME_CALL && err.message().contains("subscription was started with `withRuntime` set to `false`")
+		Error::JsonRpc(ref err) if err.code() == super::error::rpc_spec_v2::INVALID_RUNTIME_CALL && err.message().contains("subscription was started with `withRuntime` set to `false`")
 	);
 }
 
@@ -627,7 +626,7 @@ async fn get_storage_hash() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
+		Error::JsonRpc(ref err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Valid call without storage at the key.
@@ -895,7 +894,7 @@ async fn get_storage_value() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
+		Error::JsonRpc(ref err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Valid call without storage at the key.
@@ -1571,7 +1570,7 @@ async fn follow_with_unpin() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
+		Error::JsonRpc(ref err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// To not exceed the number of pinned blocks, we need to unpin before the next import.
@@ -1674,7 +1673,7 @@ async fn unpin_duplicate_hashes() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_DUPLICATE_HASHES && err.message() == "Received duplicate hashes for the `chainHead_unpin` method"
+		Error::JsonRpc(err) if err.code() == super::error::rpc_spec_v2::INVALID_DUPLICATE_HASHES && err.message() == "Received duplicate hashes for the `chainHead_unpin` method"
 	);
 
 	// Block tree:
@@ -1709,7 +1708,7 @@ async fn unpin_duplicate_hashes() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_DUPLICATE_HASHES && err.message() == "Received duplicate hashes for the `chainHead_unpin` method"
+		Error::JsonRpc(err) if err.code() == super::error::rpc_spec_v2::INVALID_DUPLICATE_HASHES && err.message() == "Received duplicate hashes for the `chainHead_unpin` method"
 	);
 
 	// Can unpin blocks.
@@ -1822,7 +1821,7 @@ async fn follow_with_multiple_unpin_hashes() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
+		Error::JsonRpc(ref err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	let _res: () = api
@@ -1839,7 +1838,7 @@ async fn follow_with_multiple_unpin_hashes() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
+		Error::JsonRpc(ref err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Unpin multiple blocks.
@@ -1857,7 +1856,7 @@ async fn follow_with_multiple_unpin_hashes() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
+		Error::JsonRpc(ref err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	let err = api
@@ -1868,7 +1867,7 @@ async fn follow_with_multiple_unpin_hashes() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
+		Error::JsonRpc(ref err) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 }
 

substrate/client/rpc-spec-v2/src/transaction/tests.rs

@@ -24,7 +24,7 @@ use crate::{
 use assert_matches::assert_matches;
 use codec::Encode;
 use futures::Future;
-use jsonrpsee::{core::error::Error, rpc_params, RpcModule};
+use jsonrpsee::{rpc_params, MethodsError as Error, RpcModule};
 use sc_transaction_pool::*;
 use sc_transaction_pool_api::{ChainEvent, MaintainedTransactionPool, TransactionPool};
 use sp_core::{testing::TaskExecutor, traits::SpawnNamed};
@@ -194,7 +194,7 @@ async fn tx_broadcast_invalid_tx() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::json_rpc_spec::INVALID_PARAM_ERROR && err.message() == "Invalid params"
+		Error::JsonRpc(err) if err.code() == super::error::json_rpc_spec::INVALID_PARAM_ERROR && err.message() == "Invalid params"
 	);
 
 	assert_eq!(0, pool.status().ready);
@@ -219,7 +219,7 @@ async fn tx_broadcast_invalid_tx() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::json_rpc_spec::INVALID_PARAM_ERROR && err.message() == "Invalid operation id"
+		Error::JsonRpc(err) if err.code() == super::error::json_rpc_spec::INVALID_PARAM_ERROR && err.message() == "Invalid operation id"
 	);
 }
 
@@ -233,6 +233,6 @@ async fn tx_invalid_stop() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(err) if err.code() == super::error::json_rpc_spec::INVALID_PARAM_ERROR && err.message() == "Invalid operation id"
+		Error::JsonRpc(err) if err.code() == super::error::json_rpc_spec::INVALID_PARAM_ERROR && err.message() == "Invalid operation id"
 	);
 }