From 2bc4ed11532541fd7b9e919ba59bf712632864d7 Mon Sep 17 00:00:00 2001
From: Alexandru Gheorghe <49718502+alexggh@users.noreply.github.com>
Date: Mon, 15 Apr 2024 09:23:35 +0300
Subject: [PATCH] Prevent accidental change of network-key for active
 authorities  (#3852)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

As discovered during investigation of
https://github.com/paritytech/polkadot-sdk/issues/3314 and
https://github.com/paritytech/polkadot-sdk/issues/3673 there are active
validators which accidentally might change their network key during
restart, that's not a safe operation when you are in the active set
because of distributed nature of DHT, so the old records would still
exist in the network until they expire 36h, so unless they have a good
reason validators should avoid changing their key when they restart
their nodes.

There is an effort in parallel to improve this situation
https://github.com/paritytech/polkadot-sdk/pull/3786, but those changes
are way more intrusive and will need more rigorous testing, additionally
they will reduce the time to less than 36h, but the propagation won't be
instant anyway, so not changing your network during restart should be
the safest way to run your node, unless you have a really good reason to
change it.

## Proposal
1. Do not auto-generate the network if the network file does not exist
in the provided path. Nodes where the key file does not exist will get
the following error:
```
Error:
   0: Starting an authorithy without network key in /home/alexggh/.local/share/polkadot/chains/ksmcc3/network/secret_ed25519.

       This is not a safe operation because the old identity still lives in the dht for 36 hours.

       Because of it your node might suffer from not being properly connected to other nodes for validation purposes.

       If it is the first time running your node you could use one of the following methods.

       1. Pass --unsafe-force-node-key-generation and make sure you remove it for subsequent node restarts

       2. Separetly generate the key with: polkadot key generate-node-key --file <YOUR_PATH_TO_NODE_KEY>
```

2. Add an explicit parameters for nodes that do want to change their
network despite the warnings or if they run the node for the first time.
`--unsafe-force-node-key-generation`

3. For `polkadot key generate-node-key` add two new mutually exclusive
parameters `base_path` and `default_base_path` to help with the key
generation in the same path the polkadot main command would expect it.

4. Modify the installation scripts to auto-generate a key in default
path if one was not present already there, this should help with making
the executable work out of the box after an instalation.

## Notes

Nodes that do not have already the key persisted will fail to start
after this change, however I do consider that better than the current
situation where they start but they silently hide that they might not be
properly connected to their peers.

## TODO
- [x] Make sure only nodes that are authorities on producation chains
will be affected by this restrictions.
- [x] Proper PRDOC, to make sure node operators are aware this is
coming.

---------

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Co-authored-by: Dmitry Markin <dmitry@markin.tech>
Co-authored-by: s0me0ne-unkn0wn <48632512+s0me0ne-unkn0wn@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
---
 polkadot/tests/benchmark_block.rs             |   8 +-
 prdoc/pr_3852.prdoc                           |  25 +++
 substrate/bin/utils/subkey/src/lib.rs         |   4 +-
 .../cli/src/commands/generate_node_key.rs     | 150 +++++++++++++++---
 .../cli/src/commands/inspect_node_key.rs      |   6 +-
 substrate/client/cli/src/commands/key.rs      |   5 +-
 substrate/client/cli/src/commands/mod.rs      |   2 +-
 substrate/client/cli/src/config.rs            |  42 ++++-
 substrate/client/cli/src/error.rs             |  16 ++
 .../client/cli/src/params/node_key_params.rs  | 109 ++++++++++---
 10 files changed, 303 insertions(+), 64 deletions(-)
 create mode 100644 prdoc/pr_3852.prdoc

diff --git a/polkadot/tests/benchmark_block.rs b/polkadot/tests/benchmark_block.rs
index 99f95ef611a..bc268025985 100644
--- a/polkadot/tests/benchmark_block.rs
+++ b/polkadot/tests/benchmark_block.rs
@@ -58,7 +58,13 @@ async fn build_chain(runtime: &str, base_path: &Path) {
 	let mut cmd = Command::new(cargo_bin("polkadot"))
 		.stdout(process::Stdio::piped())
 		.stderr(process::Stdio::piped())
-		.args(["--chain", runtime, "--force-authoring", "--alice"])
+		.args([
+			"--chain",
+			runtime,
+			"--force-authoring",
+			"--alice",
+			"--unsafe-force-node-key-generation",
+		])
 		.arg("-d")
 		.arg(base_path)
 		.arg("--no-hardware-benchmarks")
diff --git a/prdoc/pr_3852.prdoc b/prdoc/pr_3852.prdoc
new file mode 100644
index 00000000000..f13e1766d51
--- /dev/null
+++ b/prdoc/pr_3852.prdoc
@@ -0,0 +1,25 @@
+title: (Breaking change)Enforce network key presence on authorities.
+
+doc:
+  - audience: Node Operator
+    description: |
+      (Breaking change) For all authority nodes, the node binary now enforces the presence 
+      of a network key, instead of auto-generating when it is absent.
+
+      Before this change, all node binaries were auto-generating the node key when it was not present,
+      that is dangerous because other nodes in the network expects a stable identity for authorities.
+
+      To prevent accidental generation of node key, we removed this behaviour and node binary will now throw
+      an error if the network key is not present and operators will receive instructions to either persist
+      their network key or explicitly generate a new one with the `polkadot key generate-node-key`.
+
+      To prevent this error on restart/upgrades node operators need to make sure their network key are always
+      persisted, if nodes already correctly persist all directories in `--base-path` then no action is needed.
+
+crates:
+  - name: sc-cli
+    bump: major
+  - name: polkadot
+    bump: major
+  - name: subkey
+    bump: minor
\ No newline at end of file
diff --git a/substrate/bin/utils/subkey/src/lib.rs b/substrate/bin/utils/subkey/src/lib.rs
index 33f28ef46a5..0ca65cd08a6 100644
--- a/substrate/bin/utils/subkey/src/lib.rs
+++ b/substrate/bin/utils/subkey/src/lib.rs
@@ -310,7 +310,7 @@
 
 use clap::Parser;
 use sc_cli::{
-	Error, GenerateCmd, GenerateNodeKeyCmd, InspectKeyCmd, InspectNodeKeyCmd, SignCmd, VanityCmd,
+	Error, GenerateCmd, GenerateKeyCmdCommon, InspectKeyCmd, InspectNodeKeyCmd, SignCmd, VanityCmd,
 	VerifyCmd,
 };
 
@@ -324,7 +324,7 @@ use sc_cli::{
 pub enum Subkey {
 	/// Generate a random node key, write it to a file or stdout and write the
 	/// corresponding peer-id to stderr
-	GenerateNodeKey(GenerateNodeKeyCmd),
+	GenerateNodeKey(GenerateKeyCmdCommon),
 
 	/// Generate a random account
 	Generate(GenerateCmd),
diff --git a/substrate/client/cli/src/commands/generate_node_key.rs b/substrate/client/cli/src/commands/generate_node_key.rs
index 43851dc1af5..bdb94eec93b 100644
--- a/substrate/client/cli/src/commands/generate_node_key.rs
+++ b/substrate/client/cli/src/commands/generate_node_key.rs
@@ -17,23 +17,19 @@
 
 //! Implementation of the `generate-node-key` subcommand
 
-use crate::Error;
-use clap::Parser;
+use crate::{build_network_key_dir_or_default, Error, NODE_KEY_ED25519_FILE};
+use clap::{Args, Parser};
 use libp2p_identity::{ed25519, Keypair};
+use sc_service::BasePath;
 use std::{
 	fs,
 	io::{self, Write},
 	path::PathBuf,
 };
 
-/// The `generate-node-key` command
-#[derive(Debug, Parser)]
-#[command(
-	name = "generate-node-key",
-	about = "Generate a random node key, write it to a file or stdout \
-		 	and write the corresponding peer-id to stderr"
-)]
-pub struct GenerateNodeKeyCmd {
+/// Common arguments accross all generate key commands, subkey and node.
+#[derive(Debug, Args, Clone)]
+pub struct GenerateKeyCmdCommon {
 	/// Name of file to save secret key to.
 	/// If not given, the secret key is printed to stdout.
 	#[arg(long)]
@@ -45,32 +41,111 @@ pub struct GenerateNodeKeyCmd {
 	bin: bool,
 }
 
-impl GenerateNodeKeyCmd {
+/// The `generate-node-key` command
+#[derive(Debug, Clone, Parser)]
+#[command(
+	name = "generate-node-key",
+	about = "Generate a random node key, write it to a file or stdout \
+		 	and write the corresponding peer-id to stderr"
+)]
+pub struct GenerateNodeKeyCmd {
+	#[clap(flatten)]
+	pub common: GenerateKeyCmdCommon,
+	/// Specify the chain specification.
+	///
+	/// It can be any of the predefined chains like dev, local, staging, polkadot, kusama.
+	#[arg(long, value_name = "CHAIN_SPEC")]
+	pub chain: Option<String>,
+	/// A directory where the key should be saved. If a key already
+	/// exists in the directory, it won't be overwritten.
+	#[arg(long, conflicts_with_all = ["file", "default_base_path"])]
+	base_path: Option<PathBuf>,
+
+	/// Save the key in the default directory. If a key already
+	/// exists in the directory, it won't be overwritten.
+	#[arg(long, conflicts_with_all = ["base_path", "file"])]
+	default_base_path: bool,
+}
+
+impl GenerateKeyCmdCommon {
 	/// Run the command
 	pub fn run(&self) -> Result<(), Error> {
-		let keypair = ed25519::Keypair::generate();
+		generate_key(&self.file, self.bin, None, &None, false, None)
+	}
+}
+
+impl GenerateNodeKeyCmd {
+	/// Run the command
+	pub fn run(&self, chain_spec_id: &str, executable_name: &String) -> Result<(), Error> {
+		generate_key(
+			&self.common.file,
+			self.common.bin,
+			Some(chain_spec_id),
+			&self.base_path,
+			self.default_base_path,
+			Some(executable_name),
+		)
+	}
+}
+
+// Utility function for generating a key based on the provided CLI arguments
+//
+// `file`  - Name of file to save secret key to
+// `bin`
+fn generate_key(
+	file: &Option<PathBuf>,
+	bin: bool,
+	chain_spec_id: Option<&str>,
+	base_path: &Option<PathBuf>,
+	default_base_path: bool,
+	executable_name: Option<&String>,
+) -> Result<(), Error> {
+	let keypair = ed25519::Keypair::generate();
 
-		let secret = keypair.secret();
+	let secret = keypair.secret();
 
-		let file_data = if self.bin {
-			secret.as_ref().to_owned()
-		} else {
-			array_bytes::bytes2hex("", secret).into_bytes()
-		};
+	let file_data = if bin {
+		secret.as_ref().to_owned()
+	} else {
+		array_bytes::bytes2hex("", secret).into_bytes()
+	};
 
-		match &self.file {
-			Some(file) => fs::write(file, file_data)?,
-			None => io::stdout().lock().write_all(&file_data)?,
-		}
+	match (file, base_path, default_base_path) {
+		(Some(file), None, false) => fs::write(file, file_data)?,
+		(None, Some(_), false) | (None, None, true) => {
+			let network_path = build_network_key_dir_or_default(
+				base_path.clone().map(BasePath::new),
+				chain_spec_id.unwrap_or_default(),
+				executable_name.ok_or(Error::Input("Executable name not provided".into()))?,
+			);
 
-		eprintln!("{}", Keypair::from(keypair).public().to_peer_id());
+			fs::create_dir_all(network_path.as_path())?;
 
-		Ok(())
+			let key_path = network_path.join(NODE_KEY_ED25519_FILE);
+			if key_path.exists() {
+				eprintln!("Skip generation, a key already exists in {:?}", key_path);
+				return Err(Error::KeyAlreadyExistsInPath(key_path));
+			} else {
+				eprintln!("Generating key in {:?}", key_path);
+				fs::write(key_path, file_data)?
+			}
+		},
+		(None, None, false) => io::stdout().lock().write_all(&file_data)?,
+		(_, _, _) => {
+			// This should not happen, arguments are marked as mutually exclusive.
+			return Err(Error::Input("Mutually exclusive arguments provided".into()));
+		},
 	}
+
+	eprintln!("{}", Keypair::from(keypair).public().to_peer_id());
+
+	Ok(())
 }
 
 #[cfg(test)]
-mod tests {
+pub mod tests {
+	use crate::DEFAULT_NETWORK_CONFIG_PATH;
+
 	use super::*;
 	use std::io::Read;
 	use tempfile::Builder;
@@ -80,9 +155,32 @@ mod tests {
 		let mut file = Builder::new().prefix("keyfile").tempfile().unwrap();
 		let file_path = file.path().display().to_string();
 		let generate = GenerateNodeKeyCmd::parse_from(&["generate-node-key", "--file", &file_path]);
-		assert!(generate.run().is_ok());
+		assert!(generate.run("test", &String::from("test")).is_ok());
 		let mut buf = String::new();
 		assert!(file.read_to_string(&mut buf).is_ok());
 		assert!(array_bytes::hex2bytes(&buf).is_ok());
 	}
+
+	#[test]
+	fn generate_node_key_base_path() {
+		let base_dir = Builder::new().prefix("keyfile").tempdir().unwrap();
+		let key_path = base_dir
+			.path()
+			.join("chains/test_id/")
+			.join(DEFAULT_NETWORK_CONFIG_PATH)
+			.join(NODE_KEY_ED25519_FILE);
+		let base_path = base_dir.path().display().to_string();
+		let generate =
+			GenerateNodeKeyCmd::parse_from(&["generate-node-key", "--base-path", &base_path]);
+		assert!(generate.run("test_id", &String::from("test")).is_ok());
+		let buf = fs::read_to_string(key_path.as_path()).unwrap();
+		assert!(array_bytes::hex2bytes(&buf).is_ok());
+
+		assert!(generate.run("test_id", &String::from("test")).is_err());
+		let new_buf = fs::read_to_string(key_path).unwrap();
+		assert_eq!(
+			array_bytes::hex2bytes(&new_buf).unwrap(),
+			array_bytes::hex2bytes(&buf).unwrap()
+		);
+	}
 }
diff --git a/substrate/client/cli/src/commands/inspect_node_key.rs b/substrate/client/cli/src/commands/inspect_node_key.rs
index 6cf025a2d11..25a0a685650 100644
--- a/substrate/client/cli/src/commands/inspect_node_key.rs
+++ b/substrate/client/cli/src/commands/inspect_node_key.rs
@@ -79,7 +79,9 @@ impl InspectNodeKeyCmd {
 
 #[cfg(test)]
 mod tests {
-	use super::{super::GenerateNodeKeyCmd, *};
+	use crate::commands::generate_node_key::GenerateNodeKeyCmd;
+
+	use super::*;
 
 	#[test]
 	fn inspect_node_key() {
@@ -87,7 +89,7 @@ mod tests {
 		let path = path.to_str().unwrap();
 		let cmd = GenerateNodeKeyCmd::parse_from(&["generate-node-key", "--file", path]);
 
-		assert!(cmd.run().is_ok());
+		assert!(cmd.run("test", &String::from("test")).is_ok());
 
 		let cmd = InspectNodeKeyCmd::parse_from(&["inspect-node-key", "--file", path]);
 		assert!(cmd.run().is_ok());
diff --git a/substrate/client/cli/src/commands/key.rs b/substrate/client/cli/src/commands/key.rs
index d49b7e4072c..52747b40462 100644
--- a/substrate/client/cli/src/commands/key.rs
+++ b/substrate/client/cli/src/commands/key.rs
@@ -47,7 +47,10 @@ impl KeySubcommand {
 	/// run the key subcommands
 	pub fn run<C: SubstrateCli>(&self, cli: &C) -> Result<(), Error> {
 		match self {
-			KeySubcommand::GenerateNodeKey(cmd) => cmd.run(),
+			KeySubcommand::GenerateNodeKey(cmd) => {
+				let chain_spec = cli.load_spec(cmd.chain.as_deref().unwrap_or(""))?;
+				cmd.run(chain_spec.id(), &C::executable_name())
+			},
 			KeySubcommand::Generate(cmd) => cmd.run(),
 			KeySubcommand::Inspect(cmd) => cmd.run(),
 			KeySubcommand::Insert(cmd) => cmd.run(cli),
diff --git a/substrate/client/cli/src/commands/mod.rs b/substrate/client/cli/src/commands/mod.rs
index 9d48d2bdf64..2d7a0dc72ff 100644
--- a/substrate/client/cli/src/commands/mod.rs
+++ b/substrate/client/cli/src/commands/mod.rs
@@ -42,7 +42,7 @@ mod verify;
 pub use self::{
 	build_spec_cmd::BuildSpecCmd, chain_info_cmd::ChainInfoCmd, check_block_cmd::CheckBlockCmd,
 	export_blocks_cmd::ExportBlocksCmd, export_state_cmd::ExportStateCmd, generate::GenerateCmd,
-	generate_node_key::GenerateNodeKeyCmd, import_blocks_cmd::ImportBlocksCmd,
+	generate_node_key::GenerateKeyCmdCommon, import_blocks_cmd::ImportBlocksCmd,
 	insert_key::InsertKeyCmd, inspect_key::InspectKeyCmd, inspect_node_key::InspectNodeKeyCmd,
 	key::KeySubcommand, purge_chain_cmd::PurgeChainCmd, revert_cmd::RevertCmd, run_cmd::RunCmd,
 	sign::SignCmd, vanity::VanityCmd, verify::VerifyCmd,
diff --git a/substrate/client/cli/src/config.rs b/substrate/client/cli/src/config.rs
index 5def9ce9b72..70a4885e5ee 100644
--- a/substrate/client/cli/src/config.rs
+++ b/substrate/client/cli/src/config.rs
@@ -428,8 +428,10 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 	/// By default this is retrieved from `NodeKeyParams` if it is available. Otherwise its
 	/// `NodeKeyConfig::default()`.
 	fn node_key(&self, net_config_dir: &PathBuf) -> Result<NodeKeyConfig> {
+		let is_dev = self.is_dev()?;
+		let role = self.role(is_dev)?;
 		self.node_key_params()
-			.map(|x| x.node_key(net_config_dir))
+			.map(|x| x.node_key(net_config_dir, role, is_dev))
 			.unwrap_or_else(|| Ok(Default::default()))
 	}
 
@@ -463,11 +465,9 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 		let is_dev = self.is_dev()?;
 		let chain_id = self.chain_id(is_dev)?;
 		let chain_spec = cli.load_spec(&chain_id)?;
-		let base_path = self
-			.base_path()?
-			.unwrap_or_else(|| BasePath::from_project("", "", &C::executable_name()));
-		let config_dir = base_path.config_dir(chain_spec.id());
-		let net_config_dir = config_dir.join(DEFAULT_NETWORK_CONFIG_PATH);
+		let base_path = base_path_or_default(self.base_path()?, &C::executable_name());
+		let config_dir = build_config_dir(&base_path, chain_spec.id());
+		let net_config_dir = build_net_config_dir(&config_dir);
 		let client_id = C::client_id();
 		let database_cache_size = self.database_cache_size()?.unwrap_or(1024);
 		let database = self.database()?.unwrap_or(
@@ -665,3 +665,33 @@ pub fn generate_node_name() -> String {
 		}
 	}
 }
+
+/// Returns the value of `base_path` or the default_path if it is None
+pub(crate) fn base_path_or_default(
+	base_path: Option<BasePath>,
+	executable_name: &String,
+) -> BasePath {
+	base_path.unwrap_or_else(|| BasePath::from_project("", "", executable_name))
+}
+
+/// Returns the default path for configuration  directory based on the chain_spec
+pub(crate) fn build_config_dir(base_path: &BasePath, chain_spec_id: &str) -> PathBuf {
+	base_path.config_dir(chain_spec_id)
+}
+
+/// Returns the default path for the network configuration inside the configuration dir
+pub(crate) fn build_net_config_dir(config_dir: &PathBuf) -> PathBuf {
+	config_dir.join(DEFAULT_NETWORK_CONFIG_PATH)
+}
+
+/// Returns the default path for the network directory starting from the provided base_path
+/// or from the default base_path.
+pub(crate) fn build_network_key_dir_or_default(
+	base_path: Option<BasePath>,
+	chain_spec_id: &str,
+	executable_name: &String,
+) -> PathBuf {
+	let config_dir =
+		build_config_dir(&base_path_or_default(base_path, executable_name), chain_spec_id);
+	build_net_config_dir(&config_dir)
+}
diff --git a/substrate/client/cli/src/error.rs b/substrate/client/cli/src/error.rs
index 6c0cfca4932..90ad048009a 100644
--- a/substrate/client/cli/src/error.rs
+++ b/substrate/client/cli/src/error.rs
@@ -18,6 +18,8 @@
 
 //! Initialization errors.
 
+use std::path::PathBuf;
+
 use sp_core::crypto;
 
 /// Result type alias for the CLI.
@@ -78,6 +80,20 @@ pub enum Error {
 
 	#[error(transparent)]
 	GlobalLoggerError(#[from] sc_tracing::logging::Error),
+
+	#[error(
+		"Starting an authorithy without network key in {0}.
+		\n This is not a safe operation because other authorities in the network may depend on your node having a stable identity.
+		\n Otherwise these other authorities may not being able to reach you.
+		\n If it is the first time running your node you could use one of the following methods:
+		\n 1. [Preferred] Separately generate the key with: <NODE_BINARY> key generate-node-key --base-path <YOUR_BASE_PATH>
+		\n 2. [Preferred] Separately generate the key with: <NODE_BINARY> key generate-node-key --file <YOUR_PATH_TO_NODE_KEY>
+		\n 3. [Preferred] Separately generate the key with: <NODE_BINARY> key generate-node-key --default-base-path
+		\n 4. [Unsafe] Pass --unsafe-force-node-key-generation and make sure you remove it for subsequent node restarts"
+	)]
+	NetworkKeyNotFound(PathBuf),
+	#[error("A network key already exists in path {0}")]
+	KeyAlreadyExistsInPath(PathBuf),
 }
 
 impl From<&str> for Error {
diff --git a/substrate/client/cli/src/params/node_key_params.rs b/substrate/client/cli/src/params/node_key_params.rs
index 53f19f58e1f..7058af19f1d 100644
--- a/substrate/client/cli/src/params/node_key_params.rs
+++ b/substrate/client/cli/src/params/node_key_params.rs
@@ -18,15 +18,16 @@
 
 use clap::Args;
 use sc_network::config::{identity::ed25519, NodeKeyConfig};
+use sc_service::Role;
 use sp_core::H256;
 use std::{path::PathBuf, str::FromStr};
 
-use crate::{arg_enums::NodeKeyType, error};
+use crate::{arg_enums::NodeKeyType, error, Error};
 
 /// The file name of the node's Ed25519 secret key inside the chain-specific
 /// network config directory, if neither `--node-key` nor `--node-key-file`
 /// is specified in combination with `--node-key-type=ed25519`.
-const NODE_KEY_ED25519_FILE: &str = "secret_ed25519";
+pub(crate) const NODE_KEY_ED25519_FILE: &str = "secret_ed25519";
 
 /// Parameters used to create the `NodeKeyConfig`, which determines the keypair
 /// used for libp2p networking.
@@ -79,22 +80,48 @@ pub struct NodeKeyParams {
 	/// the chosen type.
 	#[arg(long, value_name = "FILE")]
 	pub node_key_file: Option<PathBuf>,
+
+	/// Forces key generation if node-key-file file does not exist.
+	///
+	/// This is an unsafe feature for production networks, because as an active authority
+	/// other authorities may depend on your node having a stable identity and they might
+	/// not being able to reach you if your identity changes after entering the active set.
+	///
+	/// For minimal node downtime if no custom `node-key-file` argument is provided
+	/// the network-key is usually persisted accross nodes restarts,
+	/// in the `network` folder from directory provided in `--base-path`
+	///
+	/// Warning!! If you ever run the node with this argument, make sure
+	/// you remove it for the subsequent restarts.
+	#[arg(long)]
+	pub unsafe_force_node_key_generation: bool,
 }
 
 impl NodeKeyParams {
 	/// Create a `NodeKeyConfig` from the given `NodeKeyParams` in the context
 	/// of an optional network config storage directory.
-	pub fn node_key(&self, net_config_dir: &PathBuf) -> error::Result<NodeKeyConfig> {
+	pub fn node_key(
+		&self,
+		net_config_dir: &PathBuf,
+		role: Role,
+		is_dev: bool,
+	) -> error::Result<NodeKeyConfig> {
 		Ok(match self.node_key_type {
 			NodeKeyType::Ed25519 => {
 				let secret = if let Some(node_key) = self.node_key.as_ref() {
 					parse_ed25519_secret(node_key)?
 				} else {
-					sc_network::config::Secret::File(
-						self.node_key_file
-							.clone()
-							.unwrap_or_else(|| net_config_dir.join(NODE_KEY_ED25519_FILE)),
-					)
+					let key_path = self
+						.node_key_file
+						.clone()
+						.unwrap_or_else(|| net_config_dir.join(NODE_KEY_ED25519_FILE));
+					if !self.unsafe_force_node_key_generation &&
+						role.is_authority() && !is_dev &&
+						!key_path.exists()
+					{
+						return Err(Error::NetworkKeyNotFound(key_path))
+					}
+					sc_network::config::Secret::File(key_path)
 				};
 
 				NodeKeyConfig::Ed25519(secret)
@@ -122,7 +149,8 @@ mod tests {
 	use super::*;
 	use clap::ValueEnum;
 	use libp2p_identity::ed25519;
-	use std::fs;
+	use std::fs::{self, File};
+	use tempfile::TempDir;
 
 	#[test]
 	fn test_node_key_config_input() {
@@ -136,8 +164,9 @@ mod tests {
 					node_key_type,
 					node_key: Some(format!("{:x}", H256::from_slice(sk.as_ref()))),
 					node_key_file: None,
+					unsafe_force_node_key_generation: false,
 				};
-				params.node_key(net_config_dir).and_then(|c| match c {
+				params.node_key(net_config_dir, Role::Authority, false).and_then(|c| match c {
 					NodeKeyConfig::Ed25519(sc_network::config::Secret::Input(ref ski))
 						if node_key_type == NodeKeyType::Ed25519 && &sk[..] == ski.as_ref() =>
 						Ok(()),
@@ -156,10 +185,11 @@ mod tests {
 				node_key_type: NodeKeyType::Ed25519,
 				node_key: None,
 				node_key_file: Some(file),
+				unsafe_force_node_key_generation: false,
 			};
 
 			let node_key = params
-				.node_key(&PathBuf::from("not-used"))
+				.node_key(&PathBuf::from("not-used"), Role::Authority, false)
 				.expect("Creates node key config")
 				.into_keypair()
 				.expect("Creates node key pair");
@@ -186,29 +216,58 @@ mod tests {
 
 	#[test]
 	fn test_node_key_config_default() {
-		fn with_def_params<F>(f: F) -> error::Result<()>
+		fn with_def_params<F>(f: F, unsafe_force_node_key_generation: bool) -> error::Result<()>
 		where
 			F: Fn(NodeKeyParams) -> error::Result<()>,
 		{
 			NodeKeyType::value_variants().iter().try_for_each(|t| {
 				let node_key_type = *t;
-				f(NodeKeyParams { node_key_type, node_key: None, node_key_file: None })
+				f(NodeKeyParams {
+					node_key_type,
+					node_key: None,
+					node_key_file: None,
+					unsafe_force_node_key_generation,
+				})
 			})
 		}
 
-		fn some_config_dir(net_config_dir: &PathBuf) -> error::Result<()> {
-			with_def_params(|params| {
-				let dir = PathBuf::from(net_config_dir.clone());
-				let typ = params.node_key_type;
-				params.node_key(net_config_dir).and_then(move |c| match c {
-					NodeKeyConfig::Ed25519(sc_network::config::Secret::File(ref f))
-						if typ == NodeKeyType::Ed25519 && f == &dir.join(NODE_KEY_ED25519_FILE) =>
-						Ok(()),
-					_ => Err(error::Error::Input("Unexpected node key config".into())),
-				})
-			})
+		fn some_config_dir(
+			net_config_dir: &PathBuf,
+			unsafe_force_node_key_generation: bool,
+			role: Role,
+			is_dev: bool,
+		) -> error::Result<()> {
+			with_def_params(
+				|params| {
+					let dir = PathBuf::from(net_config_dir.clone());
+					let typ = params.node_key_type;
+					let role = role.clone();
+					params.node_key(net_config_dir, role, is_dev).and_then(move |c| match c {
+						NodeKeyConfig::Ed25519(sc_network::config::Secret::File(ref f))
+							if typ == NodeKeyType::Ed25519 &&
+								f == &dir.join(NODE_KEY_ED25519_FILE) =>
+							Ok(()),
+						_ => Err(error::Error::Input("Unexpected node key config".into())),
+					})
+				},
+				unsafe_force_node_key_generation,
+			)
 		}
 
-		assert!(some_config_dir(&PathBuf::from_str("x").unwrap()).is_ok());
+		assert!(some_config_dir(&PathBuf::from_str("x").unwrap(), false, Role::Full, false).is_ok());
+		assert!(
+			some_config_dir(&PathBuf::from_str("x").unwrap(), false, Role::Authority, true).is_ok()
+		);
+		assert!(
+			some_config_dir(&PathBuf::from_str("x").unwrap(), true, Role::Authority, false).is_ok()
+		);
+		assert!(matches!(
+			some_config_dir(&PathBuf::from_str("x").unwrap(), false, Role::Authority, false),
+			Err(Error::NetworkKeyNotFound(_))
+		));
+
+		let tempdir = TempDir::new().unwrap();
+		let _file = File::create(tempdir.path().join(NODE_KEY_ED25519_FILE)).unwrap();
+		assert!(some_config_dir(&tempdir.path().into(), false, Role::Authority, false).is_ok());
 	}
 }
-- 
GitLab