Name changes for GrandPa and Beefy notifications protocols (#10463)

* grandpa: update notif protocol name * grandpa: add chain id prefix to protocol name * grandpa: beautify protocol name handling * grandpa: prepend genesis hash to protocol name * chain-spec: add optional 'fork_id' 'fork_id' is used to uniquely identify forks of the same chain/network 'ChainSpec' trait provides default 'None' implementation, meaning this chain hasn't been forked. * grandpa: protocol_name mod instead of struct * beefy: add genesis hash prefix to protocol name * chainspec: add fork_id * grandpa: simplify protocol name * grandpa: contain protocol name building logic * beefy: contain protocol name building logic * grandpa: fix tests * fix merge damage * fix docs reference visibility Signed-off-by: acatangiu <adrian@parity.io> * Update client/finality-grandpa/src/lib.rs Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> * Update client/finality-grandpa/src/communication/mod.rs Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> * Update client/beefy/src/lib.rs Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> * Update client/beefy/src/lib.rs Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> * avoid using hash default, even for protocol names Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>
paritytech · Jan 5, 2022 · 0f070c8 · 0f070c8
1 parent e4d7a24
commit 0f070c8
Show file tree

Hide file tree

Showing 18 changed files with 164 additions and 47 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bin/node-template/node/src/chain_spec.rs b/bin/node-template/node/src/chain_spec.rs
@@ -68,6 +68,7 @@ pub fn development_config() -> Result<ChainSpec, String> {
 		None,
 		// Protocol ID
 		None,
+		None,
 		// Properties
 		None,
 		// Extensions
@@ -117,6 +118,7 @@ pub fn local_testnet_config() -> Result<ChainSpec, String> {
 		None,
 		// Properties
 		None,
+		None,
 		// Extensions
 		None,
 	))

diff --git a/bin/node-template/node/src/service.rs b/bin/node-template/node/src/service.rs
@@ -1,7 +1,7 @@
 //! Service and ServiceFactory implementation. Specialized wrapper over substrate service.
 
 use node_template_runtime::{self, opaque::Block, RuntimeApi};
-use sc_client_api::ExecutorProvider;
+use sc_client_api::{BlockBackend, ExecutorProvider};
 use sc_consensus_aura::{ImportQueueParams, SlotProportion, StartAuraParams};
 pub use sc_executor::NativeElseWasmExecutor;
 use sc_finality_grandpa::SharedVoterState;
@@ -180,8 +180,15 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError>
 				))),
 		};
 	}
+	let grandpa_protocol_name = sc_finality_grandpa::protocol_standard_name(
+		&client.block_hash(0).ok().flatten().expect("Genesis block exists; qed"),
+		&config.chain_spec,
+	);
 
-	config.network.extra_sets.push(sc_finality_grandpa::grandpa_peers_set_config());
+	config
+		.network
+		.extra_sets
+		.push(sc_finality_grandpa::grandpa_peers_set_config(grandpa_protocol_name.clone()));
 	let warp_sync = Arc::new(sc_finality_grandpa::warp_proof::NetworkProvider::new(
 		backend.clone(),
 		grandpa_link.shared_authority_set().clone(),
@@ -306,6 +313,7 @@ pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError>
 		keystore,
 		local_role: role,
 		telemetry: telemetry.as_ref().map(|x| x.handle()),
+		protocol_name: grandpa_protocol_name,
 	};
 
 	if enable_grandpa {

diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs
@@ -196,6 +196,7 @@ pub fn staging_testnet_config() -> ChainSpec {
 		),
 		None,
 		None,
+		None,
 		Default::default(),
 	)
 }
@@ -386,6 +387,7 @@ pub fn development_config() -> ChainSpec {
 		None,
 		None,
 		None,
+		None,
 		Default::default(),
 	)
 }
@@ -410,6 +412,7 @@ pub fn local_testnet_config() -> ChainSpec {
 		None,
 		None,
 		None,
+		None,
 		Default::default(),
 	)
 }
@@ -441,6 +444,7 @@ pub(crate) mod tests {
 			None,
 			None,
 			None,
+			None,
 			Default::default(),
 		)
 	}
@@ -456,6 +460,7 @@ pub(crate) mod tests {
 			None,
 			None,
 			None,
+			None,
 			Default::default(),
 		)
 	}

diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs
@@ -326,8 +326,15 @@ pub fn new_full_base(
 
 	let shared_voter_state = rpc_setup;
 	let auth_disc_publish_non_global_ips = config.network.allow_non_globals_in_dht;
+	let grandpa_protocol_name = grandpa::protocol_standard_name(
+		&client.block_hash(0).ok().flatten().expect("Genesis block exists; qed"),
+		&config.chain_spec,
+	);
 
-	config.network.extra_sets.push(grandpa::grandpa_peers_set_config());
+	config
+		.network
+		.extra_sets
+		.push(grandpa::grandpa_peers_set_config(grandpa_protocol_name.clone()));
 	let warp_sync = Arc::new(grandpa::warp_proof::NetworkProvider::new(
 		backend.clone(),
 		import_setup.1.shared_authority_set().clone(),
@@ -488,6 +495,7 @@ pub fn new_full_base(
 		keystore,
 		local_role: role,
 		telemetry: telemetry.as_ref().map(|x| x.handle()),
+		protocol_name: grandpa_protocol_name,
 	};
 
 	if enable_grandpa {

diff --git a/bin/utils/chain-spec-builder/src/main.rs b/bin/utils/chain-spec-builder/src/main.rs
@@ -155,6 +155,7 @@ fn generate_chain_spec(
 		None,
 		None,
 		None,
+		None,
 		Default::default(),
 	);
 

diff --git a/client/beefy/Cargo.toml b/client/beefy/Cargo.toml
@@ -26,6 +26,7 @@ sp-core = { version = "4.1.0-dev", path = "../../primitives/core" }
 sp-keystore = { version = "0.10.0", path = "../../primitives/keystore" }
 sp-runtime = { version = "4.0.0", path = "../../primitives/runtime" }
 
+sc-chain-spec = { version = "4.0.0-dev", path = "../../client/chain-spec" }
 sc-utils = { version = "4.0.0-dev", path = "../utils" }
 sc-client-api = { version = "4.0.0-dev", path = "../api" }
 sc-keystore = { version = "4.0.0-dev", path = "../keystore" }

diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs
@@ -39,15 +39,40 @@ mod round;
 mod worker;
 
 pub mod notification;
-
-pub const BEEFY_PROTOCOL_NAME: &str = "/paritytech/beefy/1";
+pub use beefy_protocol_name::standard_name as protocol_standard_name;
+
+pub(crate) mod beefy_protocol_name {
+	use sc_chain_spec::ChainSpec;
+
+	const NAME: &'static str = "/beefy/1";
+	/// Old names for the notifications protocol, used for backward compatibility.
+	pub(crate) const LEGACY_NAMES: [&'static str; 1] = ["/paritytech/beefy/1"];
+
+	/// Name of the notifications protocol used by BEEFY.
+	///
+	/// Must be registered towards the networking in order for BEEFY to properly function.
+	pub fn standard_name<Hash: std::fmt::Display>(
+		genesis_hash: &Hash,
+		chain_spec: &Box<dyn ChainSpec>,
+	) -> std::borrow::Cow<'static, str> {
+		let chain_prefix = match chain_spec.fork_id() {
+			Some(fork_id) => format!("/{}/{}", genesis_hash, fork_id),
+			None => format!("/{}", genesis_hash),
+		};
+		format!("{}{}", chain_prefix, NAME).into()
+	}
+}
 
 /// Returns the configuration value to put in
 /// [`sc_network::config::NetworkConfiguration::extra_sets`].
-pub fn beefy_peers_set_config() -> sc_network::config::NonDefaultSetConfig {
-	let mut cfg =
-		sc_network::config::NonDefaultSetConfig::new(BEEFY_PROTOCOL_NAME.into(), 1024 * 1024);
+/// For standard protocol name see [`beefy_protocol_name::standard_name`].
+pub fn beefy_peers_set_config(
+	protocol_name: std::borrow::Cow<'static, str>,
+) -> sc_network::config::NonDefaultSetConfig {
+	let mut cfg = sc_network::config::NonDefaultSetConfig::new(protocol_name, 1024 * 1024);
+
 	cfg.allow_non_reserved(25, 25);
+	cfg.add_fallback_names(beefy_protocol_name::LEGACY_NAMES.iter().map(|&n| n.into()).collect());
 	cfg
 }
 
@@ -101,6 +126,8 @@ where
 	pub min_block_delta: u32,
 	/// Prometheus metric registry
 	pub prometheus_registry: Option<Registry>,
+	/// Chain specific GRANDPA protocol name. See [`beefy_protocol_name::standard_name`].
+	pub protocol_name: std::borrow::Cow<'static, str>,
 }
 
 /// Start the BEEFY gadget.
@@ -122,11 +149,11 @@ where
 		signed_commitment_sender,
 		min_block_delta,
 		prometheus_registry,
+		protocol_name,
 	} = beefy_params;
 
 	let gossip_validator = Arc::new(gossip::GossipValidator::new());
-	let gossip_engine =
-		GossipEngine::new(network, BEEFY_PROTOCOL_NAME, gossip_validator.clone(), None);
+	let gossip_engine = GossipEngine::new(network, protocol_name, gossip_validator.clone(), None);
 
 	let metrics =
 		prometheus_registry.as_ref().map(metrics::Metrics::register).and_then(

diff --git a/client/chain-spec/src/chain_spec.rs b/client/chain-spec/src/chain_spec.rs
@@ -164,6 +164,7 @@ struct ClientSpec<E> {
 	boot_nodes: Vec<MultiaddrWithPeerId>,
 	telemetry_endpoints: Option<TelemetryEndpoints>,
 	protocol_id: Option<String>,
+	fork_id: Option<String>,
 	properties: Option<Properties>,
 	#[serde(flatten)]
 	extensions: E,
@@ -226,6 +227,11 @@ impl<G, E> ChainSpec<G, E> {
 		self.client_spec.protocol_id.as_deref()
 	}
 
+	/// Optional network fork identifier.
+	pub fn fork_id(&self) -> Option<&str> {
+		self.client_spec.fork_id.as_ref().map(String::as_str)
+	}
+
 	/// Additional loosly-typed properties of the chain.
 	///
 	/// Returns an empty JSON object if 'properties' not defined in config
@@ -257,6 +263,7 @@ impl<G, E> ChainSpec<G, E> {
 		boot_nodes: Vec<MultiaddrWithPeerId>,
 		telemetry_endpoints: Option<TelemetryEndpoints>,
 		protocol_id: Option<&str>,
+		fork_id: Option<&str>,
 		properties: Option<Properties>,
 		extensions: E,
 	) -> Self {
@@ -267,6 +274,7 @@ impl<G, E> ChainSpec<G, E> {
 			boot_nodes,
 			telemetry_endpoints,
 			protocol_id: protocol_id.map(str::to_owned),
+			fork_id: fork_id.map(str::to_owned),
 			properties,
 			extensions,
 			consensus_engine: (),
@@ -384,6 +392,10 @@ where
 		ChainSpec::protocol_id(self)
 	}
 
+	fn fork_id(&self) -> Option<&str> {
+		ChainSpec::fork_id(self)
+	}
+
 	fn properties(&self) -> Properties {
 		ChainSpec::properties(self)
 	}

diff --git a/client/chain-spec/src/lib.rs b/client/chain-spec/src/lib.rs
@@ -165,6 +165,8 @@ pub trait ChainSpec: BuildStorage + Send + Sync {
 	fn telemetry_endpoints(&self) -> &Option<TelemetryEndpoints>;
 	/// Network protocol id.
 	fn protocol_id(&self) -> Option<&str>;
+	/// Optional network fork identifier. `None` by default.
+	fn fork_id(&self) -> Option<&str>;
 	/// Additional loosly-typed properties of the chain.
 	///
 	/// Returns an empty JSON object if 'properties' not defined in config

diff --git a/client/cli/src/commands/insert_key.rs b/client/cli/src/commands/insert_key.rs
@@ -144,6 +144,7 @@ mod tests {
 				None,
 				None,
 				None,
+				None,
 				NoExtension::None,
 			)))
 		}

diff --git a/client/finality-grandpa/Cargo.toml b/client/finality-grandpa/Cargo.toml
@@ -27,6 +27,7 @@ parity-scale-codec = { version = "2.3.1", features = ["derive"] }
 sp-application-crypto = { version = "4.0.0", path = "../../primitives/application-crypto" }
 sp-arithmetic = { version = "4.0.0", path = "../../primitives/arithmetic" }
 sp-runtime = { version = "4.0.0", path = "../../primitives/runtime" }
+sc-chain-spec = { version = "4.0.0-dev", path = "../../client/chain-spec" }
 sc-utils = { version = "4.0.0-dev", path = "../utils" }
 sp-consensus = { version = "0.10.0-dev", path = "../../primitives/consensus/common" }
 sc-consensus = { version = "0.10.0-dev", path = "../consensus/common" }