Upgrade jsonrpc to 0.18.0 (#9547)

* Upgrade jsonrpc to 0.18.0

I think this says all :P

* 🤦

* Fmt etc

* Fix tests

* Fix tests again...

* Better impl

* Revert "Tell dependabot to ignore jsonrpc-* updates (#9518)"

This reverts commit 6e0cd5587d.

substrate/client/rpc/src/chain/chain_full.rs

@@ -18,19 +18,16 @@
 
 //! Blockchain API backend for full nodes.
 
+use super::{client_err, error::FutureResult, ChainBackend};
+use futures::FutureExt;
 use jsonrpc_pubsub::manager::SubscriptionManager;
-use rpc::futures::future::result;
-use std::sync::Arc;
-
 use sc_client_api::{BlockBackend, BlockchainEvents};
+use sp_blockchain::HeaderBackend;
 use sp_runtime::{
 	generic::{BlockId, SignedBlock},
 	traits::Block as BlockT,
 };
-
-use super::{client_err, error::FutureResult, ChainBackend};
-use sp_blockchain::HeaderBackend;
-use std::marker::PhantomData;
+use std::{marker::PhantomData, sync::Arc};
 
 /// Blockchain API backend for full nodes. Reads all the data from local database.
 pub struct FullChain<Block: BlockT, Client> {
@@ -52,6 +49,7 @@ impl<Block: BlockT, Client> FullChain<Block, Client> {
 impl<Block, Client> ChainBackend<Client, Block> for FullChain<Block, Client>
 where
 	Block: BlockT + 'static,
+	Block::Header: Unpin,
 	Client: BlockBackend<Block> + HeaderBackend<Block> + BlockchainEvents<Block> + 'static,
 {
 	fn client(&self) -> &Arc<Client> {
@@ -63,14 +61,12 @@ where
 	}
 
 	fn header(&self, hash: Option<Block::Hash>) -> FutureResult<Option<Block::Header>> {
-		Box::new(result(
-			self.client.header(BlockId::Hash(self.unwrap_or_best(hash))).map_err(client_err),
-		))
+		let res = self.client.header(BlockId::Hash(self.unwrap_or_best(hash))).map_err(client_err);
+		async move { res }.boxed()
 	}
 
 	fn block(&self, hash: Option<Block::Hash>) -> FutureResult<Option<SignedBlock<Block>>> {
-		Box::new(result(
-			self.client.block(&BlockId::Hash(self.unwrap_or_best(hash))).map_err(client_err),
-		))
+		let res = self.client.block(&BlockId::Hash(self.unwrap_or_best(hash))).map_err(client_err);
+		async move { res }.boxed()
 	}
 }

substrate/client/rpc/src/chain/chain_light.rs

@@ -20,7 +20,6 @@
 
 use futures::{future::ready, FutureExt, TryFutureExt};
 use jsonrpc_pubsub::manager::SubscriptionManager;
-use rpc::futures::future::{result, Either, Future};
 use std::sync::Arc;
 
 use sc_client_api::light::{Fetcher, RemoteBlockchain, RemoteBodyRequest};
@@ -61,6 +60,7 @@ impl<Block: BlockT, Client, F: Fetcher<Block>> LightChain<Block, Client, F> {
 impl<Block, Client, F> ChainBackend<Client, Block> for LightChain<Block, Client, F>
 where
 	Block: BlockT + 'static,
+	Block::Header: Unpin,
 	Client: BlockchainEvents<Block> + HeaderBackend<Block> + Send + Sync + 'static,
 	F: Fetcher<Block> + Send + Sync + 'static,
 {
@@ -82,33 +82,33 @@ where
 			BlockId::Hash(hash),
 		);
 
-		Box::new(
-			maybe_header
-				.then(move |result| ready(result.map_err(client_err)))
-				.boxed()
-				.compat(),
-		)
+		maybe_header.then(move |result| ready(result.map_err(client_err))).boxed()
 	}
 
 	fn block(&self, hash: Option<Block::Hash>) -> FutureResult<Option<SignedBlock<Block>>> {
 		let fetcher = self.fetcher.clone();
-		let block = self.header(hash).and_then(move |header| match header {
-			Some(header) => Either::A(
-				fetcher
-					.remote_body(RemoteBodyRequest {
-						header: header.clone(),
-						retry_count: Default::default(),
-					})
-					.boxed()
-					.compat()
-					.map(move |body| {
-						Some(SignedBlock { block: Block::new(header, body), justifications: None })
-					})
-					.map_err(client_err),
-			),
-			None => Either::B(result(Ok(None))),
-		});
+		self.header(hash)
+			.and_then(move |header| async move {
+				match header {
+					Some(header) => {
+						let body = fetcher
+							.remote_body(RemoteBodyRequest {
+								header: header.clone(),
+								retry_count: Default::default(),
+							})
+							.await;
 
-		Box::new(block)
+						body.map(|body| {
+							Some(SignedBlock {
+								block: Block::new(header, body),
+								justifications: None,
+							})
+						})
+						.map_err(client_err)
+					},
+					None => Ok(None),
+				}
+			})
+			.boxed()
 	}
 }

substrate/client/rpc/src/chain/mod.rs

@@ -27,7 +27,7 @@ mod tests;
 use futures::{future, StreamExt, TryStreamExt};
 use log::warn;
 use rpc::{
-	futures::{stream, Future, Sink, Stream},
+	futures::{stream, FutureExt, SinkExt, Stream},
 	Result as RpcResult,
 };
 use std::sync::Arc;
@@ -53,6 +53,7 @@ use sp_blockchain::HeaderBackend;
 trait ChainBackend<Client, Block: BlockT>: Send + Sync + 'static
 where
 	Block: BlockT + 'static,
+	Block::Header: Unpin,
 	Client: HeaderBackend<Block> + BlockchainEvents<Block> + 'static,
 {
 	/// Get client reference.
@@ -120,8 +121,7 @@ where
 			|| {
 				self.client()
 					.import_notification_stream()
-					.map(|notification| Ok::<_, ()>(notification.header))
-					.compat()
+					.map(|notification| Ok::<_, rpc::Error>(notification.header))
 			},
 		)
 	}
@@ -150,8 +150,7 @@ where
 				self.client()
 					.import_notification_stream()
 					.filter(|notification| future::ready(notification.is_new_best))
-					.map(|notification| Ok::<_, ()>(notification.header))
-					.compat()
+					.map(|notification| Ok::<_, rpc::Error>(notification.header))
 			},
 		)
 	}
@@ -179,8 +178,7 @@ where
 			|| {
 				self.client()
 					.finality_notification_stream()
-					.map(|notification| Ok::<_, ()>(notification.header))
-					.compat()
+					.map(|notification| Ok::<_, rpc::Error>(notification.header))
 			},
 		)
 	}
@@ -202,6 +200,7 @@ pub fn new_full<Block: BlockT, Client>(
 ) -> Chain<Block, Client>
 where
 	Block: BlockT + 'static,
+	Block::Header: Unpin,
 	Client: BlockBackend<Block> + HeaderBackend<Block> + BlockchainEvents<Block> + 'static,
 {
 	Chain { backend: Box::new(self::chain_full::FullChain::new(client, subscriptions)) }
@@ -216,6 +215,7 @@ pub fn new_light<Block: BlockT, Client, F: Fetcher<Block>>(
 ) -> Chain<Block, Client>
 where
 	Block: BlockT + 'static,
+	Block::Header: Unpin,
 	Client: BlockBackend<Block> + HeaderBackend<Block> + BlockchainEvents<Block> + 'static,
 	F: Send + Sync + 'static,
 {
@@ -238,6 +238,7 @@ impl<Block, Client> ChainApi<NumberFor<Block>, Block::Hash, Block::Header, Signe
 	for Chain<Block, Client>
 where
 	Block: BlockT + 'static,
+	Block::Header: Unpin,
 	Client: HeaderBackend<Block> + BlockchainEvents<Block> + 'static,
 {
 	type Metadata = crate::Metadata;
@@ -312,7 +313,7 @@ where
 }
 
 /// Subscribe to new headers.
-fn subscribe_headers<Block, Client, F, G, S, ERR>(
+fn subscribe_headers<Block, Client, F, G, S>(
 	client: &Arc<Client>,
 	subscriptions: &SubscriptionManager,
 	subscriber: Subscriber<Block::Header>,
@@ -320,27 +321,28 @@ fn subscribe_headers<Block, Client, F, G, S, ERR>(
 	stream: F,
 ) where
 	Block: BlockT + 'static,
+	Block::Header: Unpin,
 	Client: HeaderBackend<Block> + 'static,
 	F: FnOnce() -> S,
 	G: FnOnce() -> Block::Hash,
-	ERR: ::std::fmt::Debug,
-	S: Stream<Item = Block::Header, Error = ERR> + Send + 'static,
+	S: Stream<Item = std::result::Result<Block::Header, rpc::Error>> + Send + 'static,
 {
 	subscriptions.add(subscriber, |sink| {
 		// send current head right at the start.
 		let header = client
 			.header(BlockId::Hash(best_block_hash()))
 			.map_err(client_err)
-			.and_then(|header| header.ok_or_else(|| "Best header missing.".to_owned().into()))
+			.and_then(|header| header.ok_or_else(|| "Best header missing.".to_string().into()))
 			.map_err(Into::into);
 
 		// send further subscriptions
 		let stream = stream()
-			.map(|res| Ok(res))
-			.map_err(|e| warn!("Block notification stream error: {:?}", e));
+			.inspect_err(|e| warn!("Block notification stream error: {:?}", e))
+			.map(|res| Ok(res));
 
-		sink.sink_map_err(|e| warn!("Error sending notifications: {:?}", e))
-			.send_all(stream::iter_result(vec![Ok(header)]).chain(stream))
+		stream::iter(vec![Ok(header)])
+			.chain(stream)
+			.forward(sink.sink_map_err(|e| warn!("Error sending notifications: {:?}", e)))
 			// we ignore the resulting Stream (if the first stream is over we are unsubscribed)
 			.map(|_| ())
 	});

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

@@ -19,10 +19,7 @@
 use super::*;
 use crate::testing::TaskExecutor;
 use assert_matches::assert_matches;
-use futures::{
-	compat::{Future01CompatExt, Stream01CompatExt},
-	executor,
-};
+use futures::executor;
 use sc_block_builder::BlockBuilderProvider;
 use sp_consensus::BlockOrigin;
 use sp_rpc::list::ListOrValue;
@@ -37,7 +34,7 @@ fn should_return_header() {
 	let api = new_full(client.clone(), SubscriptionManager::new(Arc::new(TaskExecutor)));
 
 	assert_matches!(
-		api.header(Some(client.genesis_hash()).into()).wait(),
+		executor::block_on(api.header(Some(client.genesis_hash()).into())),
 		Ok(Some(ref x)) if x == &Header {
 			parent_hash: H256::from_low_u64_be(0),
 			number: 0,
@@ -49,7 +46,7 @@ fn should_return_header() {
 	);
 
 	assert_matches!(
-		api.header(None.into()).wait(),
+		executor::block_on(api.header(None.into())),
 		Ok(Some(ref x)) if x == &Header {
 			parent_hash: H256::from_low_u64_be(0),
 			number: 0,
@@ -60,7 +57,10 @@ fn should_return_header() {
 		}
 	);
 
-	assert_matches!(api.header(Some(H256::from_low_u64_be(5)).into()).wait(), Ok(None));
+	assert_matches!(
+		executor::block_on(api.header(Some(H256::from_low_u64_be(5)).into())),
+		Ok(None)
+	);
 }
 
 #[test]
@@ -74,12 +74,12 @@ fn should_return_a_block() {
 
 	// Genesis block is not justified
 	assert_matches!(
-		api.block(Some(client.genesis_hash()).into()).wait(),
+		executor::block_on(api.block(Some(client.genesis_hash()).into())),
 		Ok(Some(SignedBlock { justifications: None, .. }))
 	);
 
 	assert_matches!(
-		api.block(Some(block_hash).into()).wait(),
+		executor::block_on(api.block(Some(block_hash).into())),
 		Ok(Some(ref x)) if x.block == Block {
 			header: Header {
 				parent_hash: client.genesis_hash(),
@@ -94,7 +94,7 @@ fn should_return_a_block() {
 	);
 
 	assert_matches!(
-		api.block(None.into()).wait(),
+		executor::block_on(api.block(None.into())),
 		Ok(Some(ref x)) if x.block == Block {
 			header: Header {
 				parent_hash: client.genesis_hash(),
@@ -108,7 +108,7 @@ fn should_return_a_block() {
 		}
 	);
 
-	assert_matches!(api.block(Some(H256::from_low_u64_be(5)).into()).wait(), Ok(None));
+	assert_matches!(executor::block_on(api.block(Some(H256::from_low_u64_be(5)).into())), Ok(None));
 }
 
 #[test]
@@ -182,7 +182,7 @@ fn should_return_finalized_hash() {
 
 #[test]
 fn should_notify_about_latest_block() {
-	let (subscriber, id, transport) = Subscriber::new_test("test");
+	let (subscriber, id, mut transport) = Subscriber::new_test("test");
 
 	{
 		let mut client = Arc::new(substrate_test_runtime_client::new());
@@ -191,25 +191,20 @@ fn should_notify_about_latest_block() {
 		api.subscribe_all_heads(Default::default(), subscriber);
 
 		// assert id assigned
-		assert!(matches!(executor::block_on(id.compat()), Ok(Ok(SubscriptionId::String(_)))));
+		assert!(matches!(executor::block_on(id), Ok(Ok(SubscriptionId::String(_)))));
 
 		let block = client.new_block(Default::default()).unwrap().build().unwrap().block;
 		executor::block_on(client.import(BlockOrigin::Own, block)).unwrap();
 	}
 
-	// assert initial head sent.
-	let (notification, next) = executor::block_on(transport.into_future().compat()).unwrap();
-	assert!(notification.is_some());
-	// assert notification sent to transport
-	let (notification, next) = executor::block_on(next.into_future().compat()).unwrap();
-	assert!(notification.is_some());
-	// no more notifications on this channel
-	assert_eq!(executor::block_on(next.into_future().compat()).unwrap().0, None);
+	// Check for the correct number of notifications
+	executor::block_on((&mut transport).take(2).collect::<Vec<_>>());
+	assert!(executor::block_on(transport.next()).is_none());
 }
 
 #[test]
 fn should_notify_about_best_block() {
-	let (subscriber, id, transport) = Subscriber::new_test("test");
+	let (subscriber, id, mut transport) = Subscriber::new_test("test");
 
 	{
 		let mut client = Arc::new(substrate_test_runtime_client::new());
@@ -218,25 +213,20 @@ fn should_notify_about_best_block() {
 		api.subscribe_new_heads(Default::default(), subscriber);
 
 		// assert id assigned
-		assert!(matches!(executor::block_on(id.compat()), Ok(Ok(SubscriptionId::String(_)))));
+		assert!(matches!(executor::block_on(id), Ok(Ok(SubscriptionId::String(_)))));
 
 		let block = client.new_block(Default::default()).unwrap().build().unwrap().block;
 		executor::block_on(client.import(BlockOrigin::Own, block)).unwrap();
 	}
 
-	// assert initial head sent.
-	let (notification, next) = executor::block_on(transport.into_future().compat()).unwrap();
-	assert!(notification.is_some());
-	// assert notification sent to transport
-	let (notification, next) = executor::block_on(next.into_future().compat()).unwrap();
-	assert!(notification.is_some());
-	// no more notifications on this channel
-	assert_eq!(executor::block_on(Stream01CompatExt::compat(next).into_future()).0, None);
+	// Assert that the correct number of notifications have been sent.
+	executor::block_on((&mut transport).take(2).collect::<Vec<_>>());
+	assert!(executor::block_on(transport.next()).is_none());
 }
 
 #[test]
 fn should_notify_about_finalized_block() {
-	let (subscriber, id, transport) = Subscriber::new_test("test");
+	let (subscriber, id, mut transport) = Subscriber::new_test("test");
 
 	{
 		let mut client = Arc::new(substrate_test_runtime_client::new());
@@ -245,19 +235,14 @@ fn should_notify_about_finalized_block() {
 		api.subscribe_finalized_heads(Default::default(), subscriber);
 
 		// assert id assigned
-		assert!(matches!(executor::block_on(id.compat()), Ok(Ok(SubscriptionId::String(_)))));
+		assert!(matches!(executor::block_on(id), Ok(Ok(SubscriptionId::String(_)))));
 
 		let block = client.new_block(Default::default()).unwrap().build().unwrap().block;
 		executor::block_on(client.import(BlockOrigin::Own, block)).unwrap();
 		client.finalize_block(BlockId::number(1), None).unwrap();
 	}
 
-	// assert initial head sent.
-	let (notification, next) = executor::block_on(transport.into_future().compat()).unwrap();
-	assert!(notification.is_some());
-	// assert notification sent to transport
-	let (notification, next) = executor::block_on(next.into_future().compat()).unwrap();
-	assert!(notification.is_some());
-	// no more notifications on this channel
-	assert_eq!(executor::block_on(next.into_future().compat()).unwrap().0, None);
+	// Assert that the correct number of notifications have been sent.
+	executor::block_on((&mut transport).take(2).collect::<Vec<_>>());
+	assert!(executor::block_on(transport.next()).is_none());
 }