Make queryStorage and storagePairs unsafe RPC functions (#7342)

The RPC calls can be rather expensive and can easily bring a RPC node in
some problems ;)

Author: bkchr

client/rpc-api/src/state/error.rs

@@ -51,6 +51,8 @@ pub enum Error {
 		/// Maximum allowed value
 		max: u32,
 	},
+	/// Call to an unsafe RPC was denied.
+	UnsafeRpcCalled(crate::policy::UnsafeRpcError),
 }
 
 impl std::error::Error for Error {

client/rpc/src/state/mod.rs

@@ -28,7 +28,7 @@ use std::sync::Arc;
 use jsonrpc_pubsub::{typed::Subscriber, SubscriptionId, manager::SubscriptionManager};
 use rpc::{Result as RpcResult, futures::{Future, future::result}};
 
-use sc_rpc_api::state::ReadProof;
+use sc_rpc_api::{DenyUnsafe, state::ReadProof};
 use sc_client_api::light::{RemoteBlockchain, Fetcher};
 use sp_core::{Bytes, storage::{StorageKey, PrefixedStorageKey, StorageData, StorageChangeSet}};
 use sp_version::RuntimeVersion;
@@ -171,6 +171,7 @@ pub trait StateBackend<Block: BlockT, Client>: Send + Sync + 'static
 pub fn new_full<BE, Block: BlockT, Client>(
 	client: Arc<Client>,
 	subscriptions: SubscriptionManager,
+	deny_unsafe: DenyUnsafe,
 ) -> (State<Block, Client>, ChildState<Block, Client>)
 	where
 		Block: BlockT + 'static,
@@ -185,7 +186,7 @@ pub fn new_full<BE, Block: BlockT, Client>(
 		self::state_full::FullState::new(client.clone(), subscriptions.clone())
 	);
 	let backend = Box::new(self::state_full::FullState::new(client, subscriptions));
-	(State { backend }, ChildState { backend: child_backend })
+	(State { backend, deny_unsafe }, ChildState { backend: child_backend })
 }
 
 /// Create new state API that works on light node.
@@ -194,6 +195,7 @@ pub fn new_light<BE, Block: BlockT, Client, F: Fetcher<Block>>(
 	subscriptions: SubscriptionManager,
 	remote_blockchain: Arc<dyn RemoteBlockchain<Block>>,
 	fetcher: Arc<F>,
+	deny_unsafe: DenyUnsafe,
 ) -> (State<Block, Client>, ChildState<Block, Client>)
 	where
 		Block: BlockT + 'static,
@@ -217,12 +219,14 @@ pub fn new_light<BE, Block: BlockT, Client, F: Fetcher<Block>>(
 			remote_blockchain,
 			fetcher,
 	));
-	(State { backend }, ChildState { backend: child_backend })
+	(State { backend, deny_unsafe }, ChildState { backend: child_backend })
 }
 
 /// State API with subscriptions support.
 pub struct State<Block, Client> {
 	backend: Box<dyn StateBackend<Block, Client>>,
+	/// Whether to deny unsafe calls
+	deny_unsafe: DenyUnsafe,
 }
 
 impl<Block, Client> StateApi<Block::Hash> for State<Block, Client>
@@ -249,6 +253,10 @@ impl<Block, Client> StateApi<Block::Hash> for State<Block, Client>
 		key_prefix: StorageKey,
 		block: Option<Block::Hash>,
 	) -> FutureResult<Vec<(StorageKey, StorageData)>> {
+		if let Err(err) = self.deny_unsafe.check_if_safe() {
+			return Box::new(result(Err(err.into())))
+		}
+
 		self.backend.storage_pairs(block, key_prefix)
 	}
 
@@ -292,6 +300,10 @@ impl<Block, Client> StateApi<Block::Hash> for State<Block, Client>
 		from: Block::Hash,
 		to: Option<Block::Hash>
 	) -> FutureResult<Vec<StorageChangeSet<Block::Hash>>> {
+		if let Err(err) = self.deny_unsafe.check_if_safe() {
+			return Box::new(result(Err(err.into())))
+		}
+
 		self.backend.query_storage(from, to, keys)
 	}
 

client/rpc/src/state/tests.rs

@@ -32,6 +32,7 @@ use substrate_test_runtime_client::{
 	sp_consensus::BlockOrigin,
 	runtime,
 };
+use sc_rpc_api::DenyUnsafe;
 use sp_runtime::generic::BlockId;
 use crate::testing::TaskExecutor;
 use futures::{executor, compat::Future01CompatExt};
@@ -58,7 +59,11 @@ fn should_return_storage() {
 		.add_extra_storage(b":map:acc2".to_vec(), vec![1, 2, 3])
 		.build();
 	let genesis_hash = client.genesis_hash();
-	let (client, child) = new_full(Arc::new(client), SubscriptionManager::new(Arc::new(TaskExecutor)));
+	let (client, child) = new_full(
+		Arc::new(client),
+		SubscriptionManager::new(Arc::new(TaskExecutor)),
+		DenyUnsafe::No,
+	);
 	let key = StorageKey(KEY.to_vec());
 
 	assert_eq!(
@@ -96,7 +101,11 @@ fn should_return_child_storage() {
 		.add_child_storage(&child_info, "key", vec![42_u8])
 		.build());
 	let genesis_hash = client.genesis_hash();
-	let (_client, child) = new_full(client, SubscriptionManager::new(Arc::new(TaskExecutor)));
+	let (_client, child) = new_full(
+		client,
+		SubscriptionManager::new(Arc::new(TaskExecutor)),
+		DenyUnsafe::No,
+	);
 	let child_key = prefixed_storage_key();
 	let key = StorageKey(b"key".to_vec());
 
@@ -131,7 +140,11 @@ fn should_return_child_storage() {
 fn should_call_contract() {
 	let client = Arc::new(substrate_test_runtime_client::new());
 	let genesis_hash = client.genesis_hash();
-	let (client, _child) = new_full(client, SubscriptionManager::new(Arc::new(TaskExecutor)));
+	let (client, _child) = new_full(
+		client,
+		SubscriptionManager::new(Arc::new(TaskExecutor)),
+		DenyUnsafe::No,
+	);
 
 	assert_matches!(
 		client.call("balanceOf".into(), Bytes(vec![1,2,3]), Some(genesis_hash).into()).wait(),
@@ -145,7 +158,11 @@ fn should_notify_about_storage_changes() {
 
 	{
 		let mut client = Arc::new(substrate_test_runtime_client::new());
-		let (api, _child) = new_full(client.clone(), SubscriptionManager::new(Arc::new(TaskExecutor)));
+		let (api, _child) = new_full(
+			client.clone(),
+			SubscriptionManager::new(Arc::new(TaskExecutor)),
+			DenyUnsafe::No,
+		);
 
 		api.subscribe_storage(Default::default(), subscriber, None.into());
 
@@ -179,7 +196,11 @@ fn should_send_initial_storage_changes_and_notifications() {
 
 	{
 		let mut client = Arc::new(substrate_test_runtime_client::new());
-		let (api, _child) = new_full(client.clone(), SubscriptionManager::new(Arc::new(TaskExecutor)));
+		let (api, _child) = new_full(
+			client.clone(),
+			SubscriptionManager::new(Arc::new(TaskExecutor)),
+			DenyUnsafe::No,
+		);
 
 		let alice_balance_key = blake2_256(&runtime::system::balance_of_key(AccountKeyring::Alice.into()));
 
@@ -217,7 +238,11 @@ fn should_send_initial_storage_changes_and_notifications() {
 #[test]
 fn should_query_storage() {
 	fn run_tests(mut client: Arc<TestClient>, has_changes_trie_config: bool) {
-		let (api, _child) = new_full(client.clone(), SubscriptionManager::new(Arc::new(TaskExecutor)));
+		let (api, _child) = new_full(
+			client.clone(),
+			SubscriptionManager::new(Arc::new(TaskExecutor)),
+			DenyUnsafe::No,
+		);
 
 		let mut add_block = |nonce| {
 			let mut builder = client.new_block(Default::default()).unwrap();
@@ -434,7 +459,11 @@ fn should_split_ranges() {
 #[test]
 fn should_return_runtime_version() {
 	let client = Arc::new(substrate_test_runtime_client::new());
-	let (api, _child) = new_full(client.clone(), SubscriptionManager::new(Arc::new(TaskExecutor)));
+	let (api, _child) = new_full(
+		client.clone(),
+		SubscriptionManager::new(Arc::new(TaskExecutor)),
+		DenyUnsafe::No,
+	);
 
 	let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\
 		\"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",3],\
@@ -457,7 +486,11 @@ fn should_notify_on_runtime_version_initially() {
 
 	{
 		let client = Arc::new(substrate_test_runtime_client::new());
-		let (api, _child) = new_full(client.clone(), SubscriptionManager::new(Arc::new(TaskExecutor)));
+		let (api, _child) = new_full(
+			client.clone(),
+			SubscriptionManager::new(Arc::new(TaskExecutor)),
+			DenyUnsafe::No,
+		);
 
 		api.subscribe_runtime_version(Default::default(), subscriber);
 

client/service/src/builder.rs

@@ -753,13 +753,18 @@ fn gen_handler<TBl, TBackend, TExPool, TRpc, TCl>(
 			subscriptions.clone(),
 			remote_blockchain.clone(),
 			on_demand,
+			deny_unsafe,
 		);
 		(chain, state, child_state)
 
 	} else {
 		// Full nodes
 		let chain = sc_rpc::chain::new_full(client.clone(), subscriptions.clone());
-		let (state, child_state) = sc_rpc::state::new_full(client.clone(), subscriptions.clone());
+		let (state, child_state) = sc_rpc::state::new_full(
+			client.clone(),
+			subscriptions.clone(),
+			deny_unsafe,
+		);
 		(chain, state, child_state)
 	};