babe: treat epoch_authorship RPC method as unsafe

bin/node/cli/src/service.rs

@@ -43,7 +43,6 @@ macro_rules! new_full_start {
 	($config:expr) => {{
 		use std::sync::Arc;
 
-		type RpcExtension = jsonrpc_core::IoHandler<sc_rpc::Metadata>;
 		let mut import_setup = None;
 		let mut rpc_setup = None;
 		let inherent_data_providers = sp_inherents::InherentDataProviders::new();
@@ -99,30 +98,46 @@ macro_rules! new_full_start {
 				import_setup = Some((block_import, grandpa_link, babe_link));
 				Ok(import_queue)
 			})?
-			.with_rpc_extensions(|builder| -> std::result::Result<RpcExtension, _> {
-				let babe_link = import_setup.as_ref().map(|s| &s.2)
-					.expect("BabeLink is present for full services or set up failed; qed.");
+			.with_rpc_extensions_builder(|builder| {
 				let grandpa_link = import_setup.as_ref().map(|s| &s.1)
 					.expect("GRANDPA LinkHalf is present for full services or set up failed; qed.");
-				let shared_authority_set = grandpa_link.shared_authority_set();
+
+				let shared_authority_set = grandpa_link.shared_authority_set().clone();
 				let shared_voter_state = grandpa::SharedVoterState::empty();
-				let deps = node_rpc::FullDeps {
-					client: builder.client().clone(),
-					pool: builder.pool(),
-					select_chain: builder.select_chain().cloned()
-						.expect("SelectChain is present for full services or set up failed; qed."),
-					babe: node_rpc::BabeDeps {
-						keystore: builder.keystore(),
-						babe_config: sc_consensus_babe::BabeLink::config(babe_link).clone(),
-						shared_epoch_changes: sc_consensus_babe::BabeLink::epoch_changes(babe_link).clone()
-					},
-					grandpa: node_rpc::GrandpaDeps {
-						shared_voter_state: shared_voter_state.clone(),
-						shared_authority_set: shared_authority_set.clone(),
-					},
-				};
-				rpc_setup = Some((shared_voter_state));
-				Ok(node_rpc::create_full(deps))
+
+				rpc_setup = Some((shared_voter_state.clone()));
+
+				let babe_link = import_setup.as_ref().map(|s| &s.2)
+					.expect("BabeLink is present for full services or set up failed; qed.");
+
+				let babe_config = babe_link.config().clone();
+				let shared_epoch_changes = babe_link.epoch_changes().clone();
+
+				let client = builder.client().clone();
+				let pool = builder.pool().clone();
+				let select_chain = builder.select_chain().cloned()
+					.expect("SelectChain is present for full services or set up failed; qed.");
+				let keystore = builder.keystore().clone();
+
+				Ok(move |deny_unsafe| {
+					let deps = node_rpc::FullDeps {
+						client: client.clone(),
+						pool: pool.clone(),
+						select_chain: select_chain.clone(),
+						deny_unsafe,
+						babe: node_rpc::BabeDeps {
+							babe_config: babe_config.clone(),
+							shared_epoch_changes: shared_epoch_changes.clone(),
+							keystore: keystore.clone(),
+						},
+						grandpa: node_rpc::GrandpaDeps {
+							shared_voter_state: shared_voter_state.clone(),
+							shared_authority_set: shared_authority_set.clone(),
+						},
+					};
+
+					node_rpc::create_full(deps)
+				})
 			})?;
 
 		(builder, import_setup, inherent_data_providers, rpc_setup)
@@ -302,7 +317,6 @@ pub fn new_full(config: Configuration)
 /// Builds a new service for a light client.
 pub fn new_light(config: Configuration)
 -> Result<impl AbstractService, ServiceError> {
-	type RpcExtension = jsonrpc_core::IoHandler<sc_rpc::Metadata>;
 	let inherent_data_providers = InherentDataProviders::new();
 
 	let service = ServiceBuilder::new_light::<Block, RuntimeApi, node_executor::Executor>(config)?
@@ -366,9 +380,7 @@ pub fn new_light(config: Configuration)
 			let provider = client as Arc<dyn StorageAndProofProvider<_, _>>;
 			Ok(Arc::new(GrandpaFinalityProofProvider::new(backend, provider)) as _)
 		})?
-		.with_rpc_extensions(|builder,| ->
-			Result<RpcExtension, _>
-		{
+		.with_rpc_extensions(|builder| {
 			let fetcher = builder.fetcher()
 				.ok_or_else(|| "Trying to start node RPC without active fetcher")?;
 			let remote_blockchain = builder.remote_backend()
@@ -380,6 +392,7 @@ pub fn new_light(config: Configuration)
 				client: builder.client().clone(),
 				pool: builder.pool(),
 			};
+
 			Ok(node_rpc::create_light(light_deps))
 		})?
 		.build()?;

bin/node/rpc/Cargo.toml

@@ -30,3 +30,4 @@ sp-consensus = { version = "0.8.0-dev", path = "../../../primitives/consensus/co
 sp-blockchain = { version = "2.0.0-dev", path = "../../../primitives/blockchain" }
 sc-finality-grandpa = { version = "0.8.0-dev", path = "../../../client/finality-grandpa" }
 sc-finality-grandpa-rpc = { version = "0.8.0-dev", path = "../../../client/finality-grandpa/rpc" }
+sc-rpc-api = { version = "0.8.0-dev", path = "../../../client/rpc-api" }

bin/node/rpc/src/lib.rs

@@ -42,9 +42,10 @@ use sc_keystore::KeyStorePtr;
 use sp_consensus_babe::BabeApi;
 use sc_consensus_epochs::SharedEpochChanges;
 use sc_consensus_babe::{Config, Epoch};
-use sc_consensus_babe_rpc::BabeRPCHandler;
+use sc_consensus_babe_rpc::BabeRpcHandler;
 use sc_finality_grandpa::{SharedVoterState, SharedAuthoritySet};
 use sc_finality_grandpa_rpc::GrandpaRpcHandler;
+use sc_rpc_api::DenyUnsafe;
 
 /// Light client extra dependencies.
 pub struct LightDeps<C, F, P> {
@@ -84,6 +85,8 @@ pub struct FullDeps<C, P, SC> {
 	pub pool: Arc<P>,
 	/// The SelectChain Strategy
 	pub select_chain: SC,
+	/// Whether to deny unsafe calls
+	pub deny_unsafe: DenyUnsafe,
 	/// BABE specific dependencies.
 	pub babe: BabeDeps,
 	/// GRANDPA specific dependencies.
@@ -115,6 +118,7 @@ pub fn create_full<C, P, M, SC>(
 		client,
 		pool,
 		select_chain,
+		deny_unsafe,
 		babe,
 		grandpa,
 	} = deps;
@@ -142,7 +146,14 @@ pub fn create_full<C, P, M, SC>(
 	);
 	io.extend_with(
 		sc_consensus_babe_rpc::BabeApi::to_delegate(
-			BabeRPCHandler::new(client, shared_epoch_changes, keystore, babe_config, select_chain)
+			BabeRpcHandler::new(
+				client,
+				shared_epoch_changes,
+				keystore,
+				babe_config,
+				select_chain,
+				deny_unsafe,
+			),
 		)
 	);
 	io.extend_with(

client/consensus/babe/rpc/Cargo.toml

@@ -13,6 +13,7 @@ targets = ["x86_64-unknown-linux-gnu"]
 
 [dependencies]
 sc-consensus-babe = { version = "0.8.0-dev", path = "../" }
+sc-rpc-api = { version = "0.8.0-dev", path = "../../../rpc-api" }
 jsonrpc-core = "14.0.3"
 jsonrpc-core-client = "14.0.5"
 jsonrpc-derive = "14.0.3"
@@ -29,7 +30,9 @@ sp-core = { version = "2.0.0-dev", path = "../../../../primitives/core" }
 sc-keystore = { version = "2.0.0-dev", path = "../../../keystore" }
 
 [dev-dependencies]
-substrate-test-runtime-client = { version = "2.0.0-dev", path = "../../../../test-utils/runtime/client" }
+sc-consensus = { version = "0.8.0-dev", path = "../../../consensus/common" }
+serde_json = "1.0.50"
 sp-application-crypto = { version = "2.0.0-dev", path = "../../../../primitives/application-crypto" }
 sp-keyring = { version = "2.0.0-dev", path = "../../../../primitives/keyring" }
+substrate-test-runtime-client = { version = "2.0.0-dev", path = "../../../../test-utils/runtime/client" }
 tempfile = "3.1.0"

client/consensus/babe/rpc/src/lib.rs

@@ -33,6 +33,7 @@ use sp_consensus_babe::{
 };
 use serde::{Deserialize, Serialize};
 use sc_keystore::KeyStorePtr;
+use sc_rpc_api::DenyUnsafe;
 use sp_api::{ProvideRuntimeApi, BlockId};
 use sp_runtime::traits::{Block as BlockT, Header as _};
 use sp_consensus::{SelectChain, Error as ConsensusError};
@@ -50,8 +51,8 @@ pub trait BabeApi {
 	fn epoch_authorship(&self) -> FutureResult<HashMap<AuthorityId, EpochAuthorship>>;
 }
 
-/// Implements the BabeRPC trait for interacting with Babe.
-pub struct BabeRPCHandler<B: BlockT, C, SC> {
+/// Implements the BabeRpc trait for interacting with Babe.
+pub struct BabeRpcHandler<B: BlockT, C, SC> {
 	/// shared reference to the client.
 	client: Arc<C>,
 	/// shared reference to EpochChanges
@@ -62,28 +63,32 @@ pub struct BabeRPCHandler<B: BlockT, C, SC> {
 	babe_config: Config,
 	/// The SelectChain strategy
 	select_chain: SC,
+	/// Whether to deny unsafe calls
+	deny_unsafe: DenyUnsafe,
 }
 
-impl<B: BlockT, C, SC> BabeRPCHandler<B, C, SC> {
+impl<B: BlockT, C, SC> BabeRpcHandler<B, C, SC> {
 	/// Creates a new instance of the BabeRpc handler.
 	pub fn new(
 		client: Arc<C>,
 		shared_epoch_changes: SharedEpochChanges<B, Epoch>,
 		keystore: KeyStorePtr,
 		babe_config: Config,
 		select_chain: SC,
+		deny_unsafe: DenyUnsafe,
 	) -> Self {
 		Self {
 			client,
 			shared_epoch_changes,
 			keystore,
 			babe_config,
 			select_chain,
+			deny_unsafe,
 		}
 	}
 }
 
-impl<B, C, SC> BabeApi for BabeRPCHandler<B, C, SC>
+impl<B, C, SC> BabeApi for BabeRpcHandler<B, C, SC>
 	where
 		B: BlockT,
 		C: ProvideRuntimeApi<B> + HeaderBackend<B> + HeaderMetadata<B, Error=BlockChainError> + 'static,
@@ -92,6 +97,10 @@ impl<B, C, SC> BabeApi for BabeRPCHandler<B, C, SC>
 		SC: SelectChain<B> + Clone + 'static,
 {
 	fn epoch_authorship(&self) -> FutureResult<HashMap<AuthorityId, EpochAuthorship>> {
+		if let Err(err) = self.deny_unsafe.check_if_safe() {
+			return Box::new(rpc_future::err(err.into()));
+		}
+
 		let (
 			babe_config,
 			keystore,
@@ -214,7 +223,10 @@ fn epoch_data<B, C, SC>(
 mod tests {
 	use super::*;
 	use substrate_test_runtime_client::{
+		runtime::Block,
+		Backend,
 		DefaultTestClientBuilderExt,
+		TestClient,
 		TestClientBuilderExt,
 		TestClientBuilder,
 	};
@@ -236,8 +248,9 @@ mod tests {
 		(keystore, keystore_path)
 	}
 
-	#[test]
-	fn rpc() {
+	fn test_babe_rpc_handler(
+		deny_unsafe: DenyUnsafe
+	) -> BabeRpcHandler<Block, TestClient, sc_consensus::LongestChain<Backend, Block>> {
 		let builder = TestClientBuilder::new();
 		let (client, longest_chain) = builder.build_with_longest_chain();
 		let client = Arc::new(client);
@@ -249,9 +262,21 @@ mod tests {
 		).expect("can initialize block-import");
 
 		let epoch_changes = link.epoch_changes().clone();
-		let select_chain = longest_chain;
 		let keystore = create_temp_keystore::<AuthorityPair>(Ed25519Keyring::Alice).0;
-		let handler = BabeRPCHandler::new(client.clone(), epoch_changes, keystore, config, select_chain);
+
+		BabeRpcHandler::new(
+			client.clone(),
+			epoch_changes,
+			keystore,
+			config,
+			longest_chain,
+			deny_unsafe,
+		)
+	}
+
+	#[test]
+	fn epoch_authorship_works() {
+		let handler = test_babe_rpc_handler(DenyUnsafe::No);
 		let mut io = IoHandler::new();
 
 		io.extend_with(BabeApi::to_delegate(handler));
@@ -260,4 +285,19 @@ mod tests {
 
 		assert_eq!(Some(response.into()), io.handle_request_sync(request));
 	}
+
+	#[test]
+	fn epoch_authorship_is_unsafe() {
+		let handler = test_babe_rpc_handler(DenyUnsafe::Yes);
+		let mut io = IoHandler::new();
+
+		io.extend_with(BabeApi::to_delegate(handler));
+		let request = r#"{"jsonrpc":"2.0","method":"babe_epochAuthorship","params": [],"id":1}"#;
+
+		let response = io.handle_request_sync(request).unwrap();
+		let mut response: serde_json::Value = serde_json::from_str(&response).unwrap();
+		let error: RpcError = serde_json::from_value(response["error"].take()).unwrap();
+
+		assert_eq!(error, RpcError::method_not_found())
+	}
 }

client/rpc-api/src/policy.rs

@@ -26,37 +26,37 @@ use jsonrpc_core as rpc;
 /// Signifies whether a potentially unsafe RPC should be denied.
 #[derive(Clone, Copy, Debug)]
 pub enum DenyUnsafe {
-    /// Denies only potentially unsafe RPCs.
-    Yes,
-    /// Allows calling every RPCs.
-    No
+	/// Denies only potentially unsafe RPCs.
+	Yes,
+	/// Allows calling every RPCs.
+	No,
 }
 
 impl DenyUnsafe {
-    /// Returns `Ok(())` if the RPCs considered unsafe are safe to call,
-    /// otherwise returns `Err(UnsafeRpcError)`.
-    pub fn check_if_safe(self) -> Result<(), UnsafeRpcError> {
-        match self {
-            DenyUnsafe::Yes => Err(UnsafeRpcError),
-            DenyUnsafe::No => Ok(())
-        }
-    }
+	/// Returns `Ok(())` if the RPCs considered unsafe are safe to call,
+	/// otherwise returns `Err(UnsafeRpcError)`.
+	pub fn check_if_safe(self) -> Result<(), UnsafeRpcError> {
+		match self {
+			DenyUnsafe::Yes => Err(UnsafeRpcError),
+			DenyUnsafe::No => Ok(()),
+		}
+	}
 }
 
 /// Signifies whether an RPC considered unsafe is denied to be called externally.
 #[derive(Debug)]
 pub struct UnsafeRpcError;
 
 impl std::fmt::Display for UnsafeRpcError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "RPC call is unsafe to be called externally")
-    }
+	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+		write!(f, "RPC call is unsafe to be called externally")
+	}
 }
 
 impl std::error::Error for UnsafeRpcError {}
 
 impl From<UnsafeRpcError> for rpc::Error {
-    fn from(_: UnsafeRpcError) -> rpc::Error {
-        rpc::Error::method_not_found()
-    }
+	fn from(_: UnsafeRpcError) -> rpc::Error {
+		rpc::Error::method_not_found()
+	}
 }