From 79cfa6fa9bbfc817e4c808c02573d1be242e4f27 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 3 Mar 2022 13:54:09 +0200 Subject: [PATCH] BEEFY and GRANDPA protocol names should use full genesis hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit std::fmt::Display shows formats as reduced hash (e.g. 0xb0a8…dafe) Use hex::encode to format full hash. Signed-off-by: acatangiu --- Cargo.lock | 4 ++ client/beefy/Cargo.toml | 2 + client/beefy/src/lib.rs | 51 +++++++++++++++++-- client/finality-grandpa/Cargo.toml | 3 ++ .../finality-grandpa/src/communication/mod.rs | 6 +-- .../src/communication/tests.rs | 43 ++++++++++++++++ 6 files changed, 103 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9cb1eae0a91fc..11b17bd442b9c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -484,6 +484,7 @@ dependencies = [ "beefy-primitives", "fnv", "futures 0.3.19", + "hex", "log 0.4.14", "parity-scale-codec", "parking_lot 0.12.0", @@ -494,6 +495,7 @@ dependencies = [ "sc-network-gossip", "sc-network-test", "sc-utils", + "serde", "sp-api", "sp-application-crypto", "sp-arithmetic", @@ -8500,6 +8502,7 @@ dependencies = [ "fork-tree", "futures 0.3.19", "futures-timer", + "hex", "log 0.4.14", "parity-scale-codec", "parking_lot 0.12.0", @@ -8514,6 +8517,7 @@ dependencies = [ "sc-network-test", "sc-telemetry", "sc-utils", + "serde", "serde_json", "sp-api", "sp-application-crypto", diff --git a/client/beefy/Cargo.toml b/client/beefy/Cargo.toml index f23b3f5dc4a64..1cd0f1fd50d80 100644 --- a/client/beefy/Cargo.toml +++ b/client/beefy/Cargo.toml @@ -10,6 +10,7 @@ description = "BEEFY Client gadget for substrate" [dependencies] fnv = "1.0.6" futures = "0.3" +hex = "0.4.2" log = "0.4" parking_lot = "0.12.0" thiserror = "1.0" @@ -39,4 +40,5 @@ beefy-primitives = { version = "4.0.0-dev", path = "../../primitives/beefy" } sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" } sc-network-test = { version = "0.8.0", path = "../network/test" } +serde = "1.0.136" strum = { version = "0.23", features = ["derive"] } diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index 9b2bf383df8ef..29d74c15dd599 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -53,13 +53,13 @@ pub(crate) mod beefy_protocol_name { /// 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( + pub fn standard_name>( genesis_hash: &Hash, chain_spec: &Box, ) -> 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), + Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id), + None => format!("/{}", hex::encode(genesis_hash)), }; format!("{}{}", chain_prefix, NAME).into() } @@ -190,3 +190,48 @@ where worker.run().await } + +#[cfg(test)] +mod tests { + use super::*; + use sc_chain_spec::{ChainSpec, GenericChainSpec}; + use serde::{Deserialize, Serialize}; + use sp_core::H256; + use sp_runtime::{BuildStorage, Storage}; + + #[derive(Debug, Serialize, Deserialize)] + struct Genesis(std::collections::BTreeMap); + impl BuildStorage for Genesis { + fn assimilate_storage(&self, storage: &mut Storage) -> Result<(), String> { + storage.top.extend( + self.0.iter().map(|(a, b)| (a.clone().into_bytes(), b.clone().into_bytes())), + ); + Ok(()) + } + } + + #[test] + fn beefy_protocol_name() { + let chain_spec = GenericChainSpec::::from_json_file(std::path::PathBuf::from( + "../chain-spec/res/chain_spec.json", + )) + .unwrap() + .cloned_box(); + + // Create protocol name using random genesis hash. + let genesis_hash = H256::random(); + let expected = format!("/{}/beefy/1", hex::encode(genesis_hash)); + let proto_name = beefy_protocol_name::standard_name(&genesis_hash, &chain_spec); + assert_eq!(proto_name.to_string(), expected); + + // Create protocol name using hardcoded genesis hash. Verify exact representation. + let genesis_hash = [ + 50, 4, 60, 123, 58, 106, 216, 246, 194, 188, 139, 193, 33, 212, 202, 171, 9, 55, 123, + 94, 8, 43, 12, 251, 187, 57, 173, 19, 188, 74, 205, 147, + ]; + let expected = + "/32043c7b3a6ad8f6c2bc8bc121d4caab09377b5e082b0cfbbb39ad13bc4acd93/beefy/1".to_string(); + let proto_name = beefy_protocol_name::standard_name(&genesis_hash, &chain_spec); + assert_eq!(proto_name.to_string(), expected); + } +} diff --git a/client/finality-grandpa/Cargo.toml b/client/finality-grandpa/Cargo.toml index 34feadac5e414..9733f35bd0e93 100644 --- a/client/finality-grandpa/Cargo.toml +++ b/client/finality-grandpa/Cargo.toml @@ -19,6 +19,7 @@ dyn-clone = "1.0" fork-tree = { version = "3.0.0", path = "../../utils/fork-tree" } futures = "0.3.19" futures-timer = "3.0.1" +hex = "0.4.2" log = "0.4.8" parking_lot = "0.12.0" rand = "0.8.4" @@ -58,5 +59,7 @@ sc-network-test = { version = "0.8.0", path = "../network/test" } sp-keyring = { version = "6.0.0", path = "../../primitives/keyring" } substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/runtime/client" } sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" } + +serde = "1.0.136" tokio = "1.15" tempfile = "3.1.0" diff --git a/client/finality-grandpa/src/communication/mod.rs b/client/finality-grandpa/src/communication/mod.rs index 809e721448bd7..9c05774fffdf6 100644 --- a/client/finality-grandpa/src/communication/mod.rs +++ b/client/finality-grandpa/src/communication/mod.rs @@ -77,13 +77,13 @@ pub mod grandpa_protocol_name { /// Name of the notifications protocol used by GRANDPA. /// /// Must be registered towards the networking in order for GRANDPA to properly function. - pub fn standard_name( + pub fn standard_name>( genesis_hash: &Hash, chain_spec: &Box, ) -> 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), + Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id), + None => format!("/{}", hex::encode(genesis_hash)), }; format!("{}{}", chain_prefix, NAME).into() } diff --git a/client/finality-grandpa/src/communication/tests.rs b/client/finality-grandpa/src/communication/tests.rs index c135f58a2eec3..e41d21fc0684e 100644 --- a/client/finality-grandpa/src/communication/tests.rs +++ b/client/finality-grandpa/src/communication/tests.rs @@ -535,3 +535,46 @@ fn peer_with_higher_view_leads_to_catch_up_request() { futures::executor::block_on(test); } + +fn local_chain_spec() -> Box { + use sc_chain_spec::{ChainSpec, GenericChainSpec}; + use serde::{Deserialize, Serialize}; + use sp_runtime::{BuildStorage, Storage}; + + #[derive(Debug, Serialize, Deserialize)] + struct Genesis(std::collections::BTreeMap); + impl BuildStorage for Genesis { + fn assimilate_storage(&self, storage: &mut Storage) -> Result<(), String> { + storage.top.extend( + self.0.iter().map(|(a, b)| (a.clone().into_bytes(), b.clone().into_bytes())), + ); + Ok(()) + } + } + let chain_spec = GenericChainSpec::::from_json_file(std::path::PathBuf::from( + "../chain-spec/res/chain_spec.json", + )) + .unwrap(); + chain_spec.cloned_box() +} + +#[test] +fn grandpa_protocol_name() { + let chain_spec = local_chain_spec(); + + // Create protocol name using random genesis hash. + let genesis_hash = sp_core::H256::random(); + let expected = format!("/{}/grandpa/1", hex::encode(genesis_hash)); + let proto_name = grandpa_protocol_name::standard_name(&genesis_hash, &chain_spec); + assert_eq!(proto_name.to_string(), expected); + + // Create protocol name using hardcoded genesis hash. Verify exact representation. + let genesis_hash = [ + 53, 79, 112, 97, 119, 217, 39, 202, 147, 138, 225, 38, 88, 182, 215, 185, 110, 88, 8, 53, + 125, 210, 158, 151, 50, 113, 102, 59, 245, 199, 221, 240, + ]; + let expected = + "/354f706177d927ca938ae12658b6d7b96e5808357dd29e973271663bf5c7ddf0/grandpa/1".to_string(); + let proto_name = grandpa_protocol_name::standard_name(&genesis_hash, &chain_spec); + assert_eq!(proto_name.to_string(), expected); +}