From c92eda9809a2977f5446775a84729adb561f4f28 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Silva?= <andre.beat@gmail.com>
Date: Tue, 29 Oct 2019 18:58:34 +0000
Subject: [PATCH] node: add sentry mode flag (#3959)

* node: add sentry mode flag

* cli: extend docs on validator and sentry modes

* service: add missing field in test Configuration

* node: Display instead of Debug when printing node role
---
 substrate/core/cli/src/lib.rs          | 17 ++++++++++++++++-
 substrate/core/cli/src/params.rs       | 24 +++++++++++++++++++++++-
 substrate/core/service/src/config.rs   |  5 +++++
 substrate/core/service/test/src/lib.rs |  1 +
 substrate/node-template/src/cli.rs     |  4 ++--
 substrate/node-template/src/service.rs | 17 +++++++++++++++--
 substrate/node/cli/src/lib.rs          |  4 ++--
 substrate/node/cli/src/service.rs      | 17 +++++++++++++++--
 8 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/substrate/core/cli/src/lib.rs b/substrate/core/cli/src/lib.rs
index b49f686ae62..a22797861b9 100644
--- a/substrate/core/cli/src/lib.rs
+++ b/substrate/core/cli/src/lib.rs
@@ -236,6 +236,17 @@ where
 	}
 }
 
+/// Returns a string displaying the node role, special casing the sentry mode
+/// (returning `SENTRY`), since the node technically has an `AUTHORITY` role but
+/// doesn't participate.
+pub fn display_role<A, B>(config: &Configuration<(), A, B>) -> String {
+	if config.sentry_mode {
+		"SENTRY".to_string()
+	} else {
+		format!("{:?}", config.roles)
+	}
+}
+
 /// Output of calling `parse_and_prepare`.
 #[must_use]
 pub enum ParseAndPrepare<'a, CC, RP> {
@@ -658,16 +669,20 @@ where
 	config.state_cache_size = cli.state_cache_size;
 
 	let is_dev = cli.shared_params.dev;
+	let is_authority = cli.validator || cli.sentry || is_dev || cli.keyring.account.is_some();
 
 	let role =
 		if cli.light {
 			service::Roles::LIGHT
-		} else if cli.validator || is_dev || cli.keyring.account.is_some() {
+		} else if is_authority {
 			service::Roles::AUTHORITY
 		} else {
 			service::Roles::FULL
 		};
 
+	// set sentry mode (i.e. act as an authority but **never** actively participate)
+	config.sentry_mode = cli.sentry;
+
 	// by default we disable pruning if the node is an authority (i.e.
 	// `ArchiveAll`), otherwise we keep state for the last 256 blocks. if the
 	// node is an authority and pruning is enabled explicitly, then we error
diff --git a/substrate/core/cli/src/params.rs b/substrate/core/cli/src/params.rs
index 44807360664..007bdabf0b7 100644
--- a/substrate/core/cli/src/params.rs
+++ b/substrate/core/cli/src/params.rs
@@ -301,9 +301,31 @@ pub struct ExecutionStrategies {
 #[derive(Debug, StructOpt, Clone)]
 pub struct RunCmd {
 	/// Enable validator mode.
-	#[structopt(long = "validator")]
+	///
+	/// The node will be started with the authority role and actively
+	/// participate in any consensus task that it can (e.g. depending on
+	/// availability of local keys).
+	#[structopt(
+		long = "validator",
+		conflicts_with_all = &[ "sentry" ]
+	)]
 	pub validator: bool,
 
+	/// Enable sentry mode.
+	///
+	/// The node will be started with the authority role and participate in
+	/// consensus tasks as an "observer", it will never actively participate
+	/// regardless of whether it could (e.g. keys are available locally). This
+	/// mode is useful as a secure proxy for validators (which would run
+	/// detached from the network), since we want this node to participate in
+	/// the full consensus protocols in order to have all needed consensus data
+	/// available to relay to private nodes.
+	#[structopt(
+		long = "sentry",
+		conflicts_with_all = &[ "validator" ]
+	)]
+	pub sentry: bool,
+
 	/// Disable GRANDPA voter when running in validator mode, otherwise disables the GRANDPA observer.
 	#[structopt(long = "no-grandpa")]
 	pub no_grandpa: bool,
diff --git a/substrate/core/service/src/config.rs b/substrate/core/service/src/config.rs
index a1ba83753f4..8abef5c572f 100644
--- a/substrate/core/service/src/config.rs
+++ b/substrate/core/service/src/config.rs
@@ -82,6 +82,10 @@ pub struct Configuration<C, G, E = NoExtension> {
 	pub default_heap_pages: Option<u64>,
 	/// Should offchain workers be executed.
 	pub offchain_worker: bool,
+	/// Sentry mode is enabled, the node's role is AUTHORITY but it should not
+	/// actively participate in consensus (i.e. no keystores should be passed to
+	/// consensus modules).
+	pub sentry_mode: bool,
 	/// Enable authoring even when offline.
 	pub force_authoring: bool,
 	/// Disable GRANDPA when running in validator mode
@@ -129,6 +133,7 @@ impl<C, G, E> Configuration<C, G, E> where
 			telemetry_external_transport: None,
 			default_heap_pages: None,
 			offchain_worker: Default::default(),
+			sentry_mode: false,
 			force_authoring: false,
 			disable_grandpa: false,
 			keystore_password: None,
diff --git a/substrate/core/service/test/src/lib.rs b/substrate/core/service/test/src/lib.rs
index 806996576ff..28ed5bfdbc6 100644
--- a/substrate/core/service/test/src/lib.rs
+++ b/substrate/core/service/test/src/lib.rs
@@ -188,6 +188,7 @@ fn node_config<G, E: Clone> (
 		telemetry_external_transport: None,
 		default_heap_pages: None,
 		offchain_worker: false,
+		sentry_mode: false,
 		force_authoring: false,
 		disable_grandpa: false,
 		dev_key_seed: key_seed,
diff --git a/substrate/node-template/src/cli.rs b/substrate/node-template/src/cli.rs
index 15c1a0486fb..ceb51504e29 100644
--- a/substrate/node-template/src/cli.rs
+++ b/substrate/node-template/src/cli.rs
@@ -3,7 +3,7 @@ use futures::{future, Future, sync::oneshot};
 use std::cell::RefCell;
 use tokio::runtime::Runtime;
 pub use substrate_cli::{VersionInfo, IntoExit, error};
-use substrate_cli::{informant, parse_and_prepare, ParseAndPrepare, NoCustom};
+use substrate_cli::{display_role, informant, parse_and_prepare, ParseAndPrepare, NoCustom};
 use substrate_service::{AbstractService, Roles as ServiceRoles, Configuration};
 use aura_primitives::sr25519::{AuthorityPair as AuraPair};
 use crate::chain_spec;
@@ -24,7 +24,7 @@ pub fn run<I, T, E>(args: I, exit: E, version: VersionInfo) -> error::Result<()>
 			info!("  by {}, 2017, 2018", version.author);
 			info!("Chain specification: {}", config.chain_spec.name());
 			info!("Node name: {}", config.name);
-			info!("Roles: {:?}", config.roles);
+			info!("Roles: {}", display_role(&config));
 			let runtime = Runtime::new().map_err(|e| format!("{:?}", e))?;
 			match config.roles {
 				ServiceRoles::LIGHT => run_until_exit(
diff --git a/substrate/node-template/src/service.rs b/substrate/node-template/src/service.rs
index c03c254da40..398795325fd 100644
--- a/substrate/node-template/src/service.rs
+++ b/substrate/node-template/src/service.rs
@@ -80,6 +80,11 @@ pub fn new_full<C: Send + Default + 'static>(config: Configuration<C, GenesisCon
 	let name = config.name.clone();
 	let disable_grandpa = config.disable_grandpa;
 
+	// sentry nodes announce themselves as authorities to the network
+	// and should run the same protocols authorities do, but it should
+	// never actively participate in any consensus process.
+	let participates_in_consensus = is_authority && !config.sentry_mode;
+
 	let (builder, mut import_setup, inherent_data_providers) = new_full_start!(config);
 
 	let (block_import, grandpa_link) =
@@ -92,7 +97,7 @@ pub fn new_full<C: Send + Default + 'static>(config: Configuration<C, GenesisCon
 		)?
 		.build()?;
 
-	if is_authority {
+	if participates_in_consensus {
 		let proposer = basic_authorship::ProposerFactory {
 			client: service.client(),
 			transaction_pool: service.transaction_pool(),
@@ -119,13 +124,21 @@ pub fn new_full<C: Send + Default + 'static>(config: Configuration<C, GenesisCon
 		service.spawn_essential_task(aura);
 	}
 
+	// if the node isn't actively participating in consensus then it doesn't
+	// need a keystore, regardless of which protocol we use below.
+	let keystore = if participates_in_consensus {
+		Some(service.keystore())
+	} else {
+		None
+	};
+
 	let grandpa_config = grandpa::Config {
 		// FIXME #1578 make this available through chainspec
 		gossip_duration: Duration::from_millis(333),
 		justification_period: 512,
 		name: Some(name),
-		keystore: Some(service.keystore()),
 		observer_enabled: true,
+		keystore,
 		is_authority,
 	};
 
diff --git a/substrate/node/cli/src/lib.rs b/substrate/node/cli/src/lib.rs
index d339952ca83..5125ba216a2 100644
--- a/substrate/node/cli/src/lib.rs
+++ b/substrate/node/cli/src/lib.rs
@@ -31,7 +31,7 @@ pub use cli::{VersionInfo, IntoExit, NoCustom, SharedParams, ExecutionStrategyPa
 use substrate_service::{AbstractService, Roles as ServiceRoles, Configuration};
 use log::info;
 use structopt::{StructOpt, clap::App};
-use cli::{AugmentClap, GetLogFilter, parse_and_prepare, ParseAndPrepare};
+use cli::{display_role, parse_and_prepare, AugmentClap, GetLogFilter, ParseAndPrepare};
 use crate::factory_impl::FactoryState;
 use transaction_factory::RuntimeAdapter;
 use client::ExecutionStrategies;
@@ -166,7 +166,7 @@ pub fn run<I, T, E>(args: I, exit: E, version: cli::VersionInfo) -> error::Resul
 			info!("  by Parity Technologies, 2017-2019");
 			info!("Chain specification: {}", config.chain_spec.name());
 			info!("Node name: {}", config.name);
-			info!("Roles: {:?}", config.roles);
+			info!("Roles: {}", display_role(&config));
 			let runtime = RuntimeBuilder::new().name_prefix("main-tokio-").build()
 				.map_err(|e| format!("{:?}", e))?;
 			match config.roles {
diff --git a/substrate/node/cli/src/service.rs b/substrate/node/cli/src/service.rs
index 842acd41ed7..47e36bd9262 100644
--- a/substrate/node/cli/src/service.rs
+++ b/substrate/node/cli/src/service.rs
@@ -124,6 +124,11 @@ macro_rules! new_full {
 			$config.disable_grandpa
 		);
 
+		// sentry nodes announce themselves as authorities to the network
+		// and should run the same protocols authorities do, but it should
+		// never actively participate in any consensus process.
+		let participates_in_consensus = is_authority && !$config.sentry_mode;
+
 		let (builder, mut import_setup, inherent_data_providers) = new_full_start!($config);
 
 		// Dht event channel from the network to the authority discovery module. Use bounded channel to ensure
@@ -145,7 +150,7 @@ macro_rules! new_full {
 
 		($with_startup_data)(&block_import, &babe_link);
 
-		if is_authority {
+		if participates_in_consensus {
 			let proposer = substrate_basic_authorship::ProposerFactory {
 				client: service.client(),
 				transaction_pool: service.transaction_pool(),
@@ -171,13 +176,21 @@ macro_rules! new_full {
 			service.spawn_essential_task(babe);
 		}
 
+		// if the node isn't actively participating in consensus then it doesn't
+		// need a keystore, regardless of which protocol we use below.
+		let keystore = if participates_in_consensus {
+			Some(service.keystore())
+		} else {
+			None
+		};
+
 		let config = grandpa::Config {
 			// FIXME #1578 make this available through chainspec
 			gossip_duration: std::time::Duration::from_millis(333),
 			justification_period: 512,
 			name: Some(name),
-			keystore: Some(service.keystore()),
 			observer_enabled: true,
+			keystore,
 			is_authority,
 		};
 
-- 
GitLab