Skip to content

Commit

Permalink
Do not enforce network key presence for collators
Browse files Browse the repository at this point in the history
Because of https://github.com/paritytech/polkadot-sdk/blob/299aacb56f4f11127b194d12692b00066e91ac92/cumulus/client/cli/src/lib.rs#L318
#3852 wrongly enforces
that the network key is present for collators started with
polkadot-parachain binary, instead of generating it
on the fly.

That is not necessary and created some small friction for
collators, since they have to pass `--unsafe-force-node-key-generation`
or generate the key with the `generate-node-key` command.

This PR fixes that since the change was intended to apply
only for relaychain authorithies.

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
  • Loading branch information
alexggh committed Jul 5, 2024
1 parent 18ed309 commit 785e912
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 13 deletions.
5 changes: 5 additions & 0 deletions polkadot/cli/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ impl SubstrateCli for Cli {
},
})
}

// Enforce the presence of a network key if the node is started as an authorithy.
fn enforce_network_key_exists_when_authority(&self) -> bool {
true
}
}

fn set_default_ss58_version(spec: &Box<dyn polkadot_service::ChainSpec>) {
Expand Down
11 changes: 8 additions & 3 deletions substrate/client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,11 +437,15 @@ 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> {
fn node_key(
&self,
net_config_dir: &PathBuf,
require_key_for_authority: bool,
) -> 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, role, is_dev))
.map(|x| x.node_key(net_config_dir, role, is_dev, require_key_for_authority))
.unwrap_or_else(|| Ok(Default::default()))
}

Expand Down Expand Up @@ -490,7 +494,8 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
Database::ParityDb
},
);
let node_key = self.node_key(&net_config_dir)?;
let node_key =
self.node_key(&net_config_dir, cli.enforce_network_key_exists_when_authority())?;
let role = self.role(is_dev)?;
let max_runtime_instances = self.max_runtime_instances()?.unwrap_or(8);
let is_validator = role.is_authority();
Expand Down
5 changes: 5 additions & 0 deletions substrate/client/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,4 +250,9 @@ pub trait SubstrateCli: Sized {
command.init(&Self::support_url(), &Self::impl_version(), logger_hook, &config)?;
Runner::new(config, tokio_runtime, signals)
}

/// Returns if a node should enforce the presence of a node key for authorities.
fn enforce_network_key_exists_when_authority(&self) -> bool {
false
}
}
24 changes: 14 additions & 10 deletions substrate/client/cli/src/params/node_key_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ impl NodeKeyParams {
net_config_dir: &PathBuf,
role: Role,
is_dev: bool,
require_key_for_authority: bool,
) -> error::Result<NodeKeyConfig> {
Ok(match self.node_key_type {
NodeKeyType::Ed25519 => {
Expand All @@ -116,8 +117,9 @@ impl NodeKeyParams {
.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()
require_key_for_authority &&
role.is_authority() &&
!is_dev && !key_path.exists()
{
return Err(Error::NetworkKeyNotFound(key_path))
}
Expand Down Expand Up @@ -166,12 +168,14 @@ mod tests {
node_key_file: None,
unsafe_force_node_key_generation: false,
};
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(()),
_ => Err(error::Error::Input("Unexpected node key config".into())),
})
params.node_key(net_config_dir, Role::Authority, false, true).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(()),
_ => Err(error::Error::Input("Unexpected node key config".into())),
},
)
})
}

Expand All @@ -189,7 +193,7 @@ mod tests {
};

let node_key = params
.node_key(&PathBuf::from("not-used"), Role::Authority, false)
.node_key(&PathBuf::from("not-used"), Role::Authority, false, true)
.expect("Creates node key config")
.into_keypair()
.expect("Creates node key pair");
Expand Down Expand Up @@ -238,7 +242,7 @@ mod tests {
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 {
params.node_key(net_config_dir, role, is_dev, true).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) =>
Expand Down

0 comments on commit 785e912

Please sign in to comment.