From 27167e1554a10f93fe45c07cdeff3828eec89a61 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Fri, 29 Jul 2022 15:07:41 +0300 Subject: [PATCH 01/10] Rename transactions protocol to include genesis hash --- client/network/src/config.rs | 6 ++++- client/network/src/service.rs | 12 ++++++++-- client/network/src/service/tests.rs | 3 +++ client/network/src/transactions.rs | 35 ++++++++++++++++++++++++++--- client/network/test/src/lib.rs | 3 +++ client/service/src/builder.rs | 1 + 6 files changed, 54 insertions(+), 6 deletions(-) diff --git a/client/network/src/config.rs b/client/network/src/config.rs index 58349fe973330..2ca34f5863016 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -87,9 +87,13 @@ where /// the network. pub transaction_pool: Arc>, - /// Name of the protocol to use on the wire. Should be different for each chain. + /// Legacy name of the protocol to use on the wire. Should be different for each chain. pub protocol_id: ProtocolId, + /// Fork ID to distinguish protocols of different hard forks. Part of the standard protocol + /// name on the wire. + pub fork_id: Option, + /// Import queue to use. /// /// The import queue is the component that verifies that blocks received from other nodes are diff --git a/client/network/src/service.rs b/client/network/src/service.rs index aeb5e25497bb3..517821a0706e3 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -224,8 +224,16 @@ where fs::create_dir_all(path)?; } - let transactions_handler_proto = - transactions::TransactionsHandlerPrototype::new(params.protocol_id.clone()); + let transactions_handler_proto = transactions::TransactionsHandlerPrototype::new( + params.protocol_id.clone(), + params + .chain + .block_hash(0u32.into()) + .ok() + .flatten() + .expect("Genesis block exists; qed"), + params.fork_id.clone(), + ); params .network_config .extra_sets diff --git a/client/network/src/service/tests.rs b/client/network/src/service/tests.rs index 181d58130aa6b..fa18073f0bfeb 100644 --- a/client/network/src/service/tests.rs +++ b/client/network/src/service/tests.rs @@ -92,6 +92,8 @@ fn build_test_full_node( let protocol_id = ProtocolId::from("/test-protocol-name"); + let fork_id = Some(String::from("test-fork-id")); + let block_request_protocol_config = { let (handler, protocol_config) = BlockRequestHandler::new(&protocol_id, client.clone(), 50); async_std::task::spawn(handler.run().boxed()); @@ -122,6 +124,7 @@ fn build_test_full_node( chain: client.clone(), transaction_pool: Arc::new(crate::config::EmptyTransactionPool), protocol_id, + fork_id, import_queue, create_chain_sync: Box::new( move |sync_mode, chain, warp_sync_provider| match ChainSync::new( diff --git a/client/network/src/transactions.rs b/client/network/src/transactions.rs index 043bdeff7ebfc..8b2eeb2e04712 100644 --- a/client/network/src/transactions.rs +++ b/client/network/src/transactions.rs @@ -70,6 +70,9 @@ const MAX_TRANSACTIONS_SIZE: u64 = 16 * 1024 * 1024; /// Maximum number of transaction validation request we keep at any moment. const MAX_PENDING_TRANSACTIONS: usize = 8192; +/// Transactions protocol name suffix (everything after /{genesis_hash}/{fork_id}...). +const PROTOCOL_NAME_SUFFIX: &str = "/transactions/1"; + mod rep { use sc_peerset::ReputationChange as Rep; /// Reputation change when a peer sends us any transaction. @@ -127,19 +130,45 @@ impl Future for PendingTransaction { /// Prototype for a [`TransactionsHandler`]. pub struct TransactionsHandlerPrototype { protocol_name: Cow<'static, str>, + fallback_protocol_names: Vec>, } impl TransactionsHandlerPrototype { /// Create a new instance. - pub fn new(protocol_id: ProtocolId) -> Self { - Self { protocol_name: format!("/{}/transactions/1", protocol_id.as_ref()).into() } + pub fn new>( + protocol_id: ProtocolId, + genesis_hash: Hash, + fork_id: Option, + ) -> Self { + Self { + protocol_name: Self::protocol_name(genesis_hash, fork_id), + fallback_protocol_names: Self::fallback_protocol_names(protocol_id), + } + } + + /// Standard transactions protocol name. + fn protocol_name>( + genesis_hash: Hash, + fork_id: Option, + ) -> Cow<'static, str> { + let chain_prefix = match fork_id { + Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id), + None => format!("/{}", hex::encode(genesis_hash)), + }; + format!("{}{}", chain_prefix, PROTOCOL_NAME_SUFFIX).into() + } + + /// Fallback (legacy) transactions protocol names. + fn fallback_protocol_names(protocol_id: ProtocolId) -> Vec> { + let fallback_name = format!("/{}/transactions/1", protocol_id.as_ref()); + std::iter::once(fallback_name.into()).collect() } /// Returns the configuration of the set to put in the network configuration. pub fn set_config(&self) -> config::NonDefaultSetConfig { config::NonDefaultSetConfig { notifications_protocol: self.protocol_name.clone(), - fallback_names: Vec::new(), + fallback_names: self.fallback_protocol_names.clone(), max_notification_size: MAX_TRANSACTIONS_SIZE, set_config: config::SetConfig { in_peers: 0, diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index d7c83810d521d..93b742bd1c533 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -805,6 +805,8 @@ where let protocol_id = ProtocolId::from("test-protocol-name"); + let fork_id = Some(String::from("test-fork-id")); + let block_request_protocol_config = { let (handler, protocol_config) = BlockRequestHandler::new(&protocol_id, client.clone(), 50); @@ -849,6 +851,7 @@ where chain: client.clone(), transaction_pool: Arc::new(EmptyTransactionPool), protocol_id, + fork_id, import_queue, create_chain_sync: Box::new(move |sync_mode, chain, warp_sync_provider| { match ChainSync::new( diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 0a18943f45006..33a0b5b5f9a6a 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -795,6 +795,7 @@ where chain: client.clone(), transaction_pool: transaction_pool_adapter as _, protocol_id, + fork_id: config.chain_spec.fork_id().map(ToOwned::to_owned), import_queue: Box::new(import_queue), create_chain_sync: Box::new( move |sync_mode, chain, warp_sync_provider| match ChainSync::new( From 56da6d5d6ff7ec6dd3255474d059bde3df19b638 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Fri, 29 Jul 2022 16:55:55 +0300 Subject: [PATCH 02/10] Add protocol name generation to sc_network::utils --- client/network/src/utils.rs | 72 +++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/client/network/src/utils.rs b/client/network/src/utils.rs index d0e61a0d0475d..f4b50bcb2189f 100644 --- a/client/network/src/utils.rs +++ b/client/network/src/utils.rs @@ -16,6 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . +use crate::config::ProtocolId; use futures::{stream::unfold, FutureExt, Stream, StreamExt}; use futures_timer::Delay; use linked_hash_set::LinkedHashSet; @@ -57,6 +58,30 @@ impl LruHashSet { } } +/// Standard protocol name on the wire based on `genesis_hash`, `fork_id`, and protocol-specific +/// `short_name`. +/// +/// `short_name` must include the leading slash, e.g.: "/transactions/1". +pub fn standard_protocol_name>( + genesis_hash: Hash, + fork_id: &Option, + short_name: &str, +) -> std::borrow::Cow<'static, str> { + let chain_prefix = match fork_id { + Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id), + None => format!("/{}", hex::encode(genesis_hash)), + }; + format!("{}{}", chain_prefix, short_name).into() +} + +/// Legacy (fallback) protocol name on the wire based on [`ProtocolId`]. +pub fn legacy_protocol_name( + protocol_id: &ProtocolId, + short_name: &str, +) -> std::borrow::Cow<'static, str> { + format!("/{}{}", protocol_id.as_ref(), short_name).into() +} + #[cfg(test)] mod tests { use super::*; @@ -82,4 +107,51 @@ mod tests { assert!(set.insert(3)); assert_eq!(vec![&1, &3], set.set.iter().collect::>()); } + + #[test] + fn standard_protocol_name_test() { + const SHORT_NAME: &str = "/protocol/1"; + + // Create protocol name using random genesis hash and no fork id. + let genesis_hash = sp_core::H256::random(); + let fork_id = None; + let expected = format!("/{}/protocol/1", hex::encode(genesis_hash)); + let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); + assert_eq!(protocol_name.to_string(), expected); + + // Create protocol name with fork id. + let fork_id = Some("fork".to_string()); + let expected = format!("/{}/fork/protocol/1", hex::encode(genesis_hash)); + let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); + assert_eq!(protocol_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 fork_id = None; + let expected = + "/354f706177d927ca938ae12658b6d7b96e5808357dd29e973271663bf5c7ddf0/protocol/1"; + let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); + assert_eq!(protocol_name.to_string(), expected.to_string()); + + // Create protocol name using hardcoded genesis hash and fork_id. + // Verify exact representation. + let fork_id = Some("fork".to_string()); + let expected = + "/354f706177d927ca938ae12658b6d7b96e5808357dd29e973271663bf5c7ddf0/fork/protocol/1"; + let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); + assert_eq!(protocol_name.to_string(), expected.to_string()); + } + + #[test] + fn legacy_protocol_name_test() { + const SHORT_NAME: &str = "/protocol/1"; + + let protocol_id = ProtocolId::from("dot"); + let expected = "/dot/protocol/1"; + let legacy_name = legacy_protocol_name(&protocol_id, SHORT_NAME); + assert_eq!(legacy_name.to_string(), expected.to_string()); + } } From 2b1381b1809f871f23cfaf9b5b123403080639a0 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Fri, 29 Jul 2022 17:09:36 +0300 Subject: [PATCH 03/10] Use utils functions for transactions protocol name generation --- client/network/src/transactions.rs | 31 ++++++++---------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/client/network/src/transactions.rs b/client/network/src/transactions.rs index 8b2eeb2e04712..823b7f8f69e66 100644 --- a/client/network/src/transactions.rs +++ b/client/network/src/transactions.rs @@ -31,7 +31,7 @@ use crate::{ error, protocol::message, service::NetworkService, - utils::{interval, LruHashSet}, + utils::{interval, legacy_protocol_name, standard_protocol_name, LruHashSet}, Event, ExHashT, ObservedRole, }; @@ -70,8 +70,8 @@ const MAX_TRANSACTIONS_SIZE: u64 = 16 * 1024 * 1024; /// Maximum number of transaction validation request we keep at any moment. const MAX_PENDING_TRANSACTIONS: usize = 8192; -/// Transactions protocol name suffix (everything after /{genesis_hash}/{fork_id}...). -const PROTOCOL_NAME_SUFFIX: &str = "/transactions/1"; +/// Transactions protocol short name (everything after /{genesis_hash}/{fork_id}...). +const PROTOCOL_SHORT_NAME: &str = "/transactions/1"; mod rep { use sc_peerset::ReputationChange as Rep; @@ -141,29 +141,14 @@ impl TransactionsHandlerPrototype { fork_id: Option, ) -> Self { Self { - protocol_name: Self::protocol_name(genesis_hash, fork_id), - fallback_protocol_names: Self::fallback_protocol_names(protocol_id), + protocol_name: standard_protocol_name(genesis_hash, &fork_id, PROTOCOL_SHORT_NAME), + fallback_protocol_names: std::iter::once( + legacy_protocol_name(&protocol_id, PROTOCOL_SHORT_NAME).into(), + ) + .collect(), } } - /// Standard transactions protocol name. - fn protocol_name>( - genesis_hash: Hash, - fork_id: Option, - ) -> Cow<'static, str> { - let chain_prefix = match fork_id { - Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id), - None => format!("/{}", hex::encode(genesis_hash)), - }; - format!("{}{}", chain_prefix, PROTOCOL_NAME_SUFFIX).into() - } - - /// Fallback (legacy) transactions protocol names. - fn fallback_protocol_names(protocol_id: ProtocolId) -> Vec> { - let fallback_name = format!("/{}/transactions/1", protocol_id.as_ref()); - std::iter::once(fallback_name.into()).collect() - } - /// Returns the configuration of the set to put in the network configuration. pub fn set_config(&self) -> config::NonDefaultSetConfig { config::NonDefaultSetConfig { From 4c1be8ca1b7c2c92203f350e8a8604f72f6a2ca9 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Fri, 29 Jul 2022 17:39:08 +0300 Subject: [PATCH 04/10] Extract protocol name generation into public module --- client/network/src/lib.rs | 1 + client/network/src/protocol_name.rs | 97 +++++++++++++++++++++++++++++ client/network/src/transactions.rs | 3 +- client/network/src/utils.rs | 72 --------------------- 4 files changed, 100 insertions(+), 73 deletions(-) create mode 100644 client/network/src/protocol_name.rs diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs index 83bc1075b8bad..10b5ddc47abb4 100644 --- a/client/network/src/lib.rs +++ b/client/network/src/lib.rs @@ -258,6 +258,7 @@ pub mod bitswap; pub mod config; pub mod error; pub mod network_state; +pub mod protocol_name; pub mod transactions; #[doc(inline)] diff --git a/client/network/src/protocol_name.rs b/client/network/src/protocol_name.rs new file mode 100644 index 0000000000000..eccfff33596c0 --- /dev/null +++ b/client/network/src/protocol_name.rs @@ -0,0 +1,97 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +//! On-the-wire protocol name generation helpers. + +use crate::config::ProtocolId; + +/// Standard protocol name on the wire based on `genesis_hash`, `fork_id`, and protocol-specific +/// `short_name`. +/// +/// `short_name` must include the leading slash, e.g.: "/transactions/1". +pub fn standard_protocol_name>( + genesis_hash: Hash, + fork_id: &Option, + short_name: &str, +) -> std::borrow::Cow<'static, str> { + let chain_prefix = match fork_id { + Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id), + None => format!("/{}", hex::encode(genesis_hash)), + }; + format!("{}{}", chain_prefix, short_name).into() +} + +/// Legacy (fallback) protocol name on the wire based on [`ProtocolId`]. +pub fn legacy_protocol_name( + protocol_id: &ProtocolId, + short_name: &str, +) -> std::borrow::Cow<'static, str> { + format!("/{}{}", protocol_id.as_ref(), short_name).into() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn standard_protocol_name_test() { + const SHORT_NAME: &str = "/protocol/1"; + + // Create protocol name using random genesis hash and no fork id. + let genesis_hash = sp_core::H256::random(); + let fork_id = None; + let expected = format!("/{}/protocol/1", hex::encode(genesis_hash)); + let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); + assert_eq!(protocol_name.to_string(), expected); + + // Create protocol name with fork id. + let fork_id = Some("fork".to_string()); + let expected = format!("/{}/fork/protocol/1", hex::encode(genesis_hash)); + let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); + assert_eq!(protocol_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 fork_id = None; + let expected = + "/354f706177d927ca938ae12658b6d7b96e5808357dd29e973271663bf5c7ddf0/protocol/1"; + let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); + assert_eq!(protocol_name.to_string(), expected.to_string()); + + // Create protocol name using hardcoded genesis hash and fork_id. + // Verify exact representation. + let fork_id = Some("fork".to_string()); + let expected = + "/354f706177d927ca938ae12658b6d7b96e5808357dd29e973271663bf5c7ddf0/fork/protocol/1"; + let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); + assert_eq!(protocol_name.to_string(), expected.to_string()); + } + + #[test] + fn legacy_protocol_name_test() { + const SHORT_NAME: &str = "/protocol/1"; + + let protocol_id = ProtocolId::from("dot"); + let expected = "/dot/protocol/1"; + let legacy_name = legacy_protocol_name(&protocol_id, SHORT_NAME); + assert_eq!(legacy_name.to_string(), expected.to_string()); + } +} \ No newline at end of file diff --git a/client/network/src/transactions.rs b/client/network/src/transactions.rs index 823b7f8f69e66..646bd2eda8654 100644 --- a/client/network/src/transactions.rs +++ b/client/network/src/transactions.rs @@ -30,8 +30,9 @@ use crate::{ config::{self, TransactionImport, TransactionImportFuture, TransactionPool}, error, protocol::message, + protocol_name::{legacy_protocol_name, standard_protocol_name}, service::NetworkService, - utils::{interval, legacy_protocol_name, standard_protocol_name, LruHashSet}, + utils::{interval, LruHashSet}, Event, ExHashT, ObservedRole, }; diff --git a/client/network/src/utils.rs b/client/network/src/utils.rs index f4b50bcb2189f..d0e61a0d0475d 100644 --- a/client/network/src/utils.rs +++ b/client/network/src/utils.rs @@ -16,7 +16,6 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::config::ProtocolId; use futures::{stream::unfold, FutureExt, Stream, StreamExt}; use futures_timer::Delay; use linked_hash_set::LinkedHashSet; @@ -58,30 +57,6 @@ impl LruHashSet { } } -/// Standard protocol name on the wire based on `genesis_hash`, `fork_id`, and protocol-specific -/// `short_name`. -/// -/// `short_name` must include the leading slash, e.g.: "/transactions/1". -pub fn standard_protocol_name>( - genesis_hash: Hash, - fork_id: &Option, - short_name: &str, -) -> std::borrow::Cow<'static, str> { - let chain_prefix = match fork_id { - Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id), - None => format!("/{}", hex::encode(genesis_hash)), - }; - format!("{}{}", chain_prefix, short_name).into() -} - -/// Legacy (fallback) protocol name on the wire based on [`ProtocolId`]. -pub fn legacy_protocol_name( - protocol_id: &ProtocolId, - short_name: &str, -) -> std::borrow::Cow<'static, str> { - format!("/{}{}", protocol_id.as_ref(), short_name).into() -} - #[cfg(test)] mod tests { use super::*; @@ -107,51 +82,4 @@ mod tests { assert!(set.insert(3)); assert_eq!(vec![&1, &3], set.set.iter().collect::>()); } - - #[test] - fn standard_protocol_name_test() { - const SHORT_NAME: &str = "/protocol/1"; - - // Create protocol name using random genesis hash and no fork id. - let genesis_hash = sp_core::H256::random(); - let fork_id = None; - let expected = format!("/{}/protocol/1", hex::encode(genesis_hash)); - let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); - assert_eq!(protocol_name.to_string(), expected); - - // Create protocol name with fork id. - let fork_id = Some("fork".to_string()); - let expected = format!("/{}/fork/protocol/1", hex::encode(genesis_hash)); - let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); - assert_eq!(protocol_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 fork_id = None; - let expected = - "/354f706177d927ca938ae12658b6d7b96e5808357dd29e973271663bf5c7ddf0/protocol/1"; - let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); - assert_eq!(protocol_name.to_string(), expected.to_string()); - - // Create protocol name using hardcoded genesis hash and fork_id. - // Verify exact representation. - let fork_id = Some("fork".to_string()); - let expected = - "/354f706177d927ca938ae12658b6d7b96e5808357dd29e973271663bf5c7ddf0/fork/protocol/1"; - let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); - assert_eq!(protocol_name.to_string(), expected.to_string()); - } - - #[test] - fn legacy_protocol_name_test() { - const SHORT_NAME: &str = "/protocol/1"; - - let protocol_id = ProtocolId::from("dot"); - let expected = "/dot/protocol/1"; - let legacy_name = legacy_protocol_name(&protocol_id, SHORT_NAME); - assert_eq!(legacy_name.to_string(), expected.to_string()); - } } From 29aa556ef947e2cfd48264db2a9fc8bc528d7ddb Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Fri, 29 Jul 2022 17:50:04 +0300 Subject: [PATCH 05/10] Use sc_network::protocol_name::standard_protocol_name() for BEEFY and GRANDPA --- client/beefy/src/lib.rs | 7 ++----- client/finality-grandpa/src/communication/mod.rs | 7 ++----- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index c025ec5686ad2..03d44596cec64 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -50,6 +50,7 @@ pub use beefy_protocol_name::standard_name as protocol_standard_name; pub(crate) mod beefy_protocol_name { use sc_chain_spec::ChainSpec; + use sc_network::protocol_name::standard_protocol_name; const NAME: &str = "/beefy/1"; /// Old names for the notifications protocol, used for backward compatibility. @@ -62,11 +63,7 @@ pub(crate) mod beefy_protocol_name { genesis_hash: &Hash, chain_spec: &Box, ) -> std::borrow::Cow<'static, str> { - let chain_prefix = match chain_spec.fork_id() { - Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id), - None => format!("/{}", hex::encode(genesis_hash)), - }; - format!("{}{}", chain_prefix, NAME).into() + standard_protocol_name(genesis_hash, &chain_spec.fork_id().map(ToOwned::to_owned), NAME) } } diff --git a/client/finality-grandpa/src/communication/mod.rs b/client/finality-grandpa/src/communication/mod.rs index 378501cffdd62..9b342147804b4 100644 --- a/client/finality-grandpa/src/communication/mod.rs +++ b/client/finality-grandpa/src/communication/mod.rs @@ -69,6 +69,7 @@ pub(crate) mod tests; pub mod grandpa_protocol_name { use sc_chain_spec::ChainSpec; + use sc_network::protocol_name::standard_protocol_name; pub(crate) const NAME: &str = "/grandpa/1"; /// Old names for the notifications protocol, used for backward compatibility. @@ -81,11 +82,7 @@ pub mod grandpa_protocol_name { genesis_hash: &Hash, chain_spec: &Box, ) -> std::borrow::Cow<'static, str> { - let chain_prefix = match chain_spec.fork_id() { - Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id), - None => format!("/{}", hex::encode(genesis_hash)), - }; - format!("{}{}", chain_prefix, NAME).into() + standard_protocol_name(genesis_hash, &chain_spec.fork_id().map(ToOwned::to_owned), NAME) } } From c46e6ecfc0f51093c72b7c1f811dfc476633db13 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Fri, 29 Jul 2022 18:12:18 +0300 Subject: [PATCH 06/10] minor: add missing newline at EOF --- client/network/src/protocol_name.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/network/src/protocol_name.rs b/client/network/src/protocol_name.rs index eccfff33596c0..05bc264ff2e52 100644 --- a/client/network/src/protocol_name.rs +++ b/client/network/src/protocol_name.rs @@ -94,4 +94,4 @@ mod tests { let legacy_name = legacy_protocol_name(&protocol_id, SHORT_NAME); assert_eq!(legacy_name.to_string(), expected.to_string()); } -} \ No newline at end of file +} From 011755d685f8ca8b85080e714437557038aec392 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Mon, 1 Aug 2022 15:35:31 +0300 Subject: [PATCH 07/10] Change block-announces protocol name to include genesis_hash & fork_id --- client/network/src/protocol.rs | 18 +++++++++++++++--- client/network/src/service.rs | 1 + 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index 3698a6b936ed5..396d7b1de9598 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -18,6 +18,7 @@ use crate::{ config, error, + protocol_name::{legacy_protocol_name, standard_protocol_name}, request_responses::RequestFailure, utils::{interval, LruHashSet}, }; @@ -93,6 +94,9 @@ const MAX_BLOCK_ANNOUNCE_SIZE: u64 = 1024 * 1024; // Must be equal to `max(MAX_BLOCK_ANNOUNCE_SIZE, MAX_TRANSACTIONS_SIZE)`. pub(crate) const BLOCK_ANNOUNCES_TRANSACTIONS_SUBSTREAM_SIZE: u64 = 16 * 1024 * 1024; +/// On-the-wire block announces protocol short name (goes after "/{genesis_hash}/{fork_id}"). +const BLOCK_ANNOUNCES_PROTOCOL_SHORT_NAME: &str = "/block-announces/1"; + /// Identifier of the peerset for the block announces protocol. const HARDCODED_PEERSETS_SYNC: sc_peerset::SetId = sc_peerset::SetId::from(0); /// Number of hardcoded peersets (the constants right above). Any set whose identifier is equal or @@ -272,6 +276,7 @@ where roles: Roles, chain: Arc, protocol_id: ProtocolId, + fork_id: &Option, network_config: &config::NetworkConfiguration, notifications_protocols_handshakes: Vec>, metrics_registry: Option<&Registry>, @@ -356,8 +361,15 @@ where sc_peerset::Peerset::from_config(sc_peerset::PeersetConfig { sets }) }; - let block_announces_protocol: Cow<'static, str> = - format!("/{}/block-announces/1", protocol_id.as_ref()).into(); + let block_announces_protocol = standard_protocol_name( + chain.block_hash(0u32.into()).ok().flatten().expect("Genesis block exists; qed"), + fork_id, + BLOCK_ANNOUNCES_PROTOCOL_SHORT_NAME, + ); + + let fallback_ba_protocol_names = + iter::once(legacy_protocol_name(&protocol_id, BLOCK_ANNOUNCES_PROTOCOL_SHORT_NAME)) + .collect(); let behaviour = { let best_number = info.best_number; @@ -370,7 +382,7 @@ where let sync_protocol_config = notifications::ProtocolConfig { name: block_announces_protocol, - fallback_names: Vec::new(), + fallback_names: fallback_ba_protocol_names, handshake: block_announces_handshake, max_notification_size: MAX_BLOCK_ANNOUNCE_SIZE, }; diff --git a/client/network/src/service.rs b/client/network/src/service.rs index 517821a0706e3..40fe3f0ac5d6c 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -266,6 +266,7 @@ where From::from(¶ms.role), params.chain.clone(), params.protocol_id.clone(), + ¶ms.fork_id, ¶ms.network_config, iter::once(Vec::new()) .chain( From 08668230558a08ed6cc1a6847e33f454bf359650 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Mon, 1 Aug 2022 20:46:22 +0300 Subject: [PATCH 08/10] Change protocol names to include genesis hash and fork id Protocols changed: - sync - state - light - sync/warp --- Cargo.lock | 2 + .../network/common/src/request_responses.rs | 3 ++ client/network/light/Cargo.toml | 1 + .../light/src/light_client_requests.rs | 23 +++++++++-- .../src/light_client_requests/handler.rs | 20 ++++++++-- client/network/src/request_responses.rs | 10 ++++- client/network/src/service/tests.rs | 8 ++-- client/network/sync/Cargo.toml | 1 + .../network/sync/src/block_request_handler.rs | 34 +++++++++++++--- .../network/sync/src/state_request_handler.rs | 40 +++++++++++++++---- .../network/sync/src/warp_request_handler.rs | 30 +++++++++++--- client/network/test/src/lib.rs | 18 ++++++--- client/service/src/builder.rs | 21 ++++++++-- 13 files changed, 171 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3b0246168632c..b26ab200fa6cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8426,6 +8426,7 @@ name = "sc-network-light" version = "0.10.0-dev" dependencies = [ "futures", + "hex", "libp2p", "log", "parity-scale-codec", @@ -8446,6 +8447,7 @@ version = "0.10.0-dev" dependencies = [ "fork-tree", "futures", + "hex", "libp2p", "log", "lru", diff --git a/client/network/common/src/request_responses.rs b/client/network/common/src/request_responses.rs index 71570e6beb864..f409d1ac16d61 100644 --- a/client/network/common/src/request_responses.rs +++ b/client/network/common/src/request_responses.rs @@ -29,6 +29,9 @@ pub struct ProtocolConfig { /// Name of the protocol on the wire. Should be something like `/foo/bar`. pub name: Cow<'static, str>, + /// Fallback on the wire protocol names to support. + pub fallback_names: Vec>, + /// Maximum allowed size, in bytes, of a request. /// /// Any request larger than this value will be declined as a way to avoid allocating too diff --git a/client/network/light/Cargo.toml b/client/network/light/Cargo.toml index 0037177fb4046..c1a0fb4759320 100644 --- a/client/network/light/Cargo.toml +++ b/client/network/light/Cargo.toml @@ -21,6 +21,7 @@ codec = { package = "parity-scale-codec", version = "3.0.0", features = [ "derive", ] } futures = "0.3.21" +hex = "0.4.0" libp2p = "0.46.1" log = "0.4.16" prost = "0.10" diff --git a/client/network/light/src/light_client_requests.rs b/client/network/light/src/light_client_requests.rs index 9eccef41e833d..b58426cf15992 100644 --- a/client/network/light/src/light_client_requests.rs +++ b/client/network/light/src/light_client_requests.rs @@ -25,16 +25,31 @@ use sc_network_common::{config::ProtocolId, request_responses::ProtocolConfig}; use std::time::Duration; -/// Generate the light client protocol name from chain specific protocol identifier. -fn generate_protocol_name(protocol_id: &ProtocolId) -> String { +/// Generate the light client protocol name from the genesis hash and fork id. +fn generate_protocol_name>(genesis_hash: Hash, fork_id: Option<&str>) -> String { + if let Some(fork_id) = fork_id { + format!("/{}/{}/light/2", hex::encode(genesis_hash), fork_id) + } else { + format!("/{}/light/2", hex::encode(genesis_hash)) + } +} + +/// Generate the legacy light client protocol name from chain specific protocol identifier. +fn generate_legacy_protocol_name(protocol_id: &ProtocolId) -> String { format!("/{}/light/2", protocol_id.as_ref()) } /// Generates a [`ProtocolConfig`] for the light client request protocol, refusing incoming /// requests. -pub fn generate_protocol_config(protocol_id: &ProtocolId) -> ProtocolConfig { +pub fn generate_protocol_config>( + protocol_id: &ProtocolId, + genesis_hash: Hash, + fork_id: Option<&str>, +) -> ProtocolConfig { ProtocolConfig { - name: generate_protocol_name(protocol_id).into(), + name: generate_protocol_name(genesis_hash, fork_id).into(), + fallback_names: std::iter::once(generate_legacy_protocol_name(protocol_id).into()) + .collect(), max_request_size: 1 * 1024 * 1024, max_response_size: 16 * 1024 * 1024, request_timeout: Duration::from_secs(15), diff --git a/client/network/light/src/light_client_requests/handler.rs b/client/network/light/src/light_client_requests/handler.rs index 3c87ccfd6ed9f..727a9b0d7e820 100644 --- a/client/network/light/src/light_client_requests/handler.rs +++ b/client/network/light/src/light_client_requests/handler.rs @@ -28,7 +28,7 @@ use futures::{channel::mpsc, prelude::*}; use libp2p::PeerId; use log::{debug, trace}; use prost::Message; -use sc_client_api::{ProofProvider, StorageProof}; +use sc_client_api::{BlockBackend, ProofProvider, StorageProof}; use sc_network_common::{ config::ProtocolId, request_responses::{IncomingRequest, OutgoingResponse, ProtocolConfig}, @@ -54,15 +54,27 @@ pub struct LightClientRequestHandler { impl LightClientRequestHandler where B: Block, - Client: ProofProvider + Send + Sync + 'static, + Client: BlockBackend + ProofProvider + Send + Sync + 'static, { /// Create a new [`LightClientRequestHandler`]. - pub fn new(protocol_id: &ProtocolId, client: Arc) -> (Self, ProtocolConfig) { + pub fn new( + protocol_id: &ProtocolId, + fork_id: Option<&str>, + client: Arc, + ) -> (Self, ProtocolConfig) { // For now due to lack of data on light client request handling in production systems, this // value is chosen to match the block request limit. let (tx, request_receiver) = mpsc::channel(20); - let mut protocol_config = super::generate_protocol_config(protocol_id); + let mut protocol_config = super::generate_protocol_config( + protocol_id, + client + .block_hash(0u32.into()) + .ok() + .flatten() + .expect("Genesis block exists; qed"), + fork_id, + ); protocol_config.inbound_queue = Some(tx); (Self { client, request_receiver, _block: PhantomData::default() }, protocol_config) diff --git a/client/network/src/request_responses.rs b/client/network/src/request_responses.rs index cec4aa2a07fba..0d8c6c33b1c95 100644 --- a/client/network/src/request_responses.rs +++ b/client/network/src/request_responses.rs @@ -220,7 +220,9 @@ impl RequestResponsesBehaviour { max_request_size: protocol.max_request_size, max_response_size: protocol.max_response_size, }, - iter::once((protocol.name.as_bytes().to_vec(), protocol_support)), + iter::once(protocol.name.as_bytes().to_vec()) + .chain(protocol.fallback_names.iter().map(|name| name.as_bytes().to_vec())) + .zip(iter::repeat(protocol_support)), cfg, ); @@ -1027,6 +1029,7 @@ mod tests { let protocol_config = ProtocolConfig { name: From::from(protocol_name), + fallback_names: Vec::new(), max_request_size: 1024, max_response_size: 1024 * 1024, request_timeout: Duration::from_secs(30), @@ -1127,6 +1130,7 @@ mod tests { let protocol_config = ProtocolConfig { name: From::from(protocol_name), + fallback_names: Vec::new(), max_request_size: 1024, max_response_size: 8, // <-- important for the test request_timeout: Duration::from_secs(30), @@ -1223,6 +1227,7 @@ mod tests { let protocol_configs = vec![ ProtocolConfig { name: From::from(protocol_name_1), + fallback_names: Vec::new(), max_request_size: 1024, max_response_size: 1024 * 1024, request_timeout: Duration::from_secs(30), @@ -1230,6 +1235,7 @@ mod tests { }, ProtocolConfig { name: From::from(protocol_name_2), + fallback_names: Vec::new(), max_request_size: 1024, max_response_size: 1024 * 1024, request_timeout: Duration::from_secs(30), @@ -1247,6 +1253,7 @@ mod tests { let protocol_configs = vec![ ProtocolConfig { name: From::from(protocol_name_1), + fallback_names: Vec::new(), max_request_size: 1024, max_response_size: 1024 * 1024, request_timeout: Duration::from_secs(30), @@ -1254,6 +1261,7 @@ mod tests { }, ProtocolConfig { name: From::from(protocol_name_2), + fallback_names: Vec::new(), max_request_size: 1024, max_response_size: 1024 * 1024, request_timeout: Duration::from_secs(30), diff --git a/client/network/src/service/tests.rs b/client/network/src/service/tests.rs index fa18073f0bfeb..86757e97e6a58 100644 --- a/client/network/src/service/tests.rs +++ b/client/network/src/service/tests.rs @@ -95,20 +95,22 @@ fn build_test_full_node( let fork_id = Some(String::from("test-fork-id")); let block_request_protocol_config = { - let (handler, protocol_config) = BlockRequestHandler::new(&protocol_id, client.clone(), 50); + let (handler, protocol_config) = + BlockRequestHandler::new(&protocol_id, None, client.clone(), 50); async_std::task::spawn(handler.run().boxed()); protocol_config }; let state_request_protocol_config = { - let (handler, protocol_config) = StateRequestHandler::new(&protocol_id, client.clone(), 50); + let (handler, protocol_config) = + StateRequestHandler::new(&protocol_id, None, client.clone(), 50); async_std::task::spawn(handler.run().boxed()); protocol_config }; let light_client_request_protocol_config = { let (handler, protocol_config) = - LightClientRequestHandler::new(&protocol_id, client.clone()); + LightClientRequestHandler::new(&protocol_id, None, client.clone()); async_std::task::spawn(handler.run().boxed()); protocol_config }; diff --git a/client/network/sync/Cargo.toml b/client/network/sync/Cargo.toml index 3e3526146400a..7c8f8adafd214 100644 --- a/client/network/sync/Cargo.toml +++ b/client/network/sync/Cargo.toml @@ -21,6 +21,7 @@ codec = { package = "parity-scale-codec", version = "3.0.0", features = [ "derive", ] } futures = "0.3.21" +hex = "0.4.0" libp2p = "0.46.1" log = "0.4.17" lru = "0.7.5" diff --git a/client/network/sync/src/block_request_handler.rs b/client/network/sync/src/block_request_handler.rs index 2a847a8bf36ec..64c1d431c8cf8 100644 --- a/client/network/sync/src/block_request_handler.rs +++ b/client/network/sync/src/block_request_handler.rs @@ -62,9 +62,15 @@ mod rep { } /// Generates a [`ProtocolConfig`] for the block request protocol, refusing incoming requests. -pub fn generate_protocol_config(protocol_id: &ProtocolId) -> ProtocolConfig { +pub fn generate_protocol_config>( + protocol_id: &ProtocolId, + genesis_hash: Hash, + fork_id: Option<&str>, +) -> ProtocolConfig { ProtocolConfig { - name: generate_protocol_name(protocol_id).into(), + name: generate_protocol_name(genesis_hash, fork_id).into(), + fallback_names: std::iter::once(generate_legacy_protocol_name(protocol_id).into()) + .collect(), max_request_size: 1024 * 1024, max_response_size: 16 * 1024 * 1024, request_timeout: Duration::from_secs(20), @@ -72,8 +78,17 @@ pub fn generate_protocol_config(protocol_id: &ProtocolId) -> ProtocolConfig { } } -/// Generate the block protocol name from chain specific protocol identifier. -fn generate_protocol_name(protocol_id: &ProtocolId) -> String { +/// Generate the block protocol name from the genesis hash and fork id. +fn generate_protocol_name>(genesis_hash: Hash, fork_id: Option<&str>) -> String { + if let Some(fork_id) = fork_id { + format!("/{}/{}/sync/2", hex::encode(genesis_hash), fork_id) + } else { + format!("/{}/sync/2", hex::encode(genesis_hash)) + } +} + +/// Generate the legacy block protocol name from chain specific protocol identifier. +fn generate_legacy_protocol_name(protocol_id: &ProtocolId) -> String { format!("/{}/sync/2", protocol_id.as_ref()) } @@ -129,6 +144,7 @@ where /// Create a new [`BlockRequestHandler`]. pub fn new( protocol_id: &ProtocolId, + fork_id: Option<&str>, client: Arc, num_peer_hint: usize, ) -> (Self, ProtocolConfig) { @@ -136,7 +152,15 @@ where // number of peers. let (tx, request_receiver) = mpsc::channel(num_peer_hint); - let mut protocol_config = generate_protocol_config(protocol_id); + let mut protocol_config = generate_protocol_config( + protocol_id, + client + .block_hash(0u32.into()) + .ok() + .flatten() + .expect("Genesis block exists; qed"), + fork_id, + ); protocol_config.inbound_queue = Some(tx); let seen_requests = LruCache::new(num_peer_hint * 2); diff --git a/client/network/sync/src/state_request_handler.rs b/client/network/sync/src/state_request_handler.rs index 8e0bae14046da..6cf6482a44f8b 100644 --- a/client/network/sync/src/state_request_handler.rs +++ b/client/network/sync/src/state_request_handler.rs @@ -27,7 +27,7 @@ use libp2p::PeerId; use log::{debug, trace}; use lru::LruCache; use prost::Message; -use sc_client_api::ProofProvider; +use sc_client_api::{BlockBackend, ProofProvider}; use sc_network_common::{ config::ProtocolId, request_responses::{IncomingRequest, OutgoingResponse, ProtocolConfig}, @@ -50,10 +50,16 @@ mod rep { pub const SAME_REQUEST: Rep = Rep::new(i32::MIN, "Same state request multiple times"); } -/// Generates a [`ProtocolConfig`] for the block request protocol, refusing incoming requests. -pub fn generate_protocol_config(protocol_id: &ProtocolId) -> ProtocolConfig { +/// Generates a [`ProtocolConfig`] for the state request protocol, refusing incoming requests. +pub fn generate_protocol_config>( + protocol_id: &ProtocolId, + genesis_hash: Hash, + fork_id: Option<&str>, +) -> ProtocolConfig { ProtocolConfig { - name: generate_protocol_name(protocol_id).into(), + name: generate_protocol_name(genesis_hash, fork_id).into(), + fallback_names: std::iter::once(generate_legacy_protocol_name(protocol_id).into()) + .collect(), max_request_size: 1024 * 1024, max_response_size: 16 * 1024 * 1024, request_timeout: Duration::from_secs(40), @@ -61,8 +67,17 @@ pub fn generate_protocol_config(protocol_id: &ProtocolId) -> ProtocolConfig { } } -/// Generate the state protocol name from chain specific protocol identifier. -fn generate_protocol_name(protocol_id: &ProtocolId) -> String { +/// Generate the state protocol name from the genesis hash and fork id. +fn generate_protocol_name>(genesis_hash: Hash, fork_id: Option<&str>) -> String { + if let Some(fork_id) = fork_id { + format!("/{}/{}/state/2", hex::encode(genesis_hash), fork_id) + } else { + format!("/{}/state/2", hex::encode(genesis_hash)) + } +} + +/// Generate the legacy state protocol name from chain specific protocol identifier. +fn generate_legacy_protocol_name(protocol_id: &ProtocolId) -> String { format!("/{}/state/2", protocol_id.as_ref()) } @@ -104,11 +119,12 @@ pub struct StateRequestHandler { impl StateRequestHandler where B: BlockT, - Client: ProofProvider + Send + Sync + 'static, + Client: BlockBackend + ProofProvider + Send + Sync + 'static, { /// Create a new [`StateRequestHandler`]. pub fn new( protocol_id: &ProtocolId, + fork_id: Option<&str>, client: Arc, num_peer_hint: usize, ) -> (Self, ProtocolConfig) { @@ -116,7 +132,15 @@ where // number of peers. let (tx, request_receiver) = mpsc::channel(num_peer_hint); - let mut protocol_config = generate_protocol_config(protocol_id); + let mut protocol_config = generate_protocol_config( + protocol_id, + client + .block_hash(0u32.into()) + .ok() + .flatten() + .expect("Genesis block exists; qed"), + fork_id, + ); protocol_config.inbound_queue = Some(tx); let seen_requests = LruCache::new(num_peer_hint * 2); diff --git a/client/network/sync/src/warp_request_handler.rs b/client/network/sync/src/warp_request_handler.rs index 53ec216a1e668..394bc68449099 100644 --- a/client/network/sync/src/warp_request_handler.rs +++ b/client/network/sync/src/warp_request_handler.rs @@ -36,9 +36,15 @@ const MAX_RESPONSE_SIZE: u64 = 16 * 1024 * 1024; /// Generates a [`RequestResponseConfig`] for the grandpa warp sync request protocol, refusing /// incoming requests. -pub fn generate_request_response_config(protocol_id: ProtocolId) -> RequestResponseConfig { +pub fn generate_request_response_config>( + protocol_id: ProtocolId, + genesis_hash: Hash, + fork_id: Option<&str>, +) -> RequestResponseConfig { RequestResponseConfig { - name: generate_protocol_name(protocol_id).into(), + name: generate_protocol_name(genesis_hash, fork_id).into(), + fallback_names: std::iter::once(generate_legacy_protocol_name(protocol_id).into()) + .collect(), max_request_size: 32, max_response_size: MAX_RESPONSE_SIZE, request_timeout: Duration::from_secs(10), @@ -46,8 +52,17 @@ pub fn generate_request_response_config(protocol_id: ProtocolId) -> RequestRespo } } -/// Generate the grandpa warp sync protocol name from chain specific protocol identifier. -fn generate_protocol_name(protocol_id: ProtocolId) -> String { +/// Generate the grandpa warp sync protocol name from the genesi hash and fork id. +fn generate_protocol_name>(genesis_hash: Hash, fork_id: Option<&str>) -> String { + if let Some(fork_id) = fork_id { + format!("/{}/{}/sync/warp", hex::encode(genesis_hash), fork_id) + } else { + format!("/{}/sync/warp", hex::encode(genesis_hash)) + } +} + +/// Generate the legacy grandpa warp sync protocol name from chain specific protocol identifier. +fn generate_legacy_protocol_name(protocol_id: ProtocolId) -> String { format!("/{}/sync/warp", protocol_id.as_ref()) } @@ -59,13 +74,16 @@ pub struct RequestHandler { impl RequestHandler { /// Create a new [`RequestHandler`]. - pub fn new( + pub fn new>( protocol_id: ProtocolId, + genesis_hash: Hash, + fork_id: Option<&str>, backend: Arc>, ) -> (Self, RequestResponseConfig) { let (tx, request_receiver) = mpsc::channel(20); - let mut request_response_config = generate_request_response_config(protocol_id); + let mut request_response_config = + generate_request_response_config(protocol_id, genesis_hash, fork_id); request_response_config.inbound_queue = Some(tx); (Self { backend, request_receiver }, request_response_config) diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index 93b742bd1c533..b1739ce4e519c 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -809,21 +809,21 @@ where let block_request_protocol_config = { let (handler, protocol_config) = - BlockRequestHandler::new(&protocol_id, client.clone(), 50); + BlockRequestHandler::new(&protocol_id, None, client.clone(), 50); self.spawn_task(handler.run().boxed()); protocol_config }; let state_request_protocol_config = { let (handler, protocol_config) = - StateRequestHandler::new(&protocol_id, client.clone(), 50); + StateRequestHandler::new(&protocol_id, None, client.clone(), 50); self.spawn_task(handler.run().boxed()); protocol_config }; let light_client_request_protocol_config = { let (handler, protocol_config) = - LightClientRequestHandler::new(&protocol_id, client.clone()); + LightClientRequestHandler::new(&protocol_id, None, client.clone()); self.spawn_task(handler.run().boxed()); protocol_config }; @@ -831,8 +831,16 @@ where let warp_sync = Arc::new(TestWarpSyncProvider(client.clone())); let warp_protocol_config = { - let (handler, protocol_config) = - warp_request_handler::RequestHandler::new(protocol_id.clone(), warp_sync.clone()); + let (handler, protocol_config) = warp_request_handler::RequestHandler::new( + protocol_id.clone(), + client + .block_hash(0u32.into()) + .ok() + .flatten() + .expect("Genesis block exists; qed"), + None, + warp_sync.clone(), + ); self.spawn_task(handler.run().boxed()); protocol_config }; diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 33a0b5b5f9a6a..2ffa31701a798 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -741,6 +741,7 @@ where // Allow both outgoing and incoming requests. let (handler, protocol_config) = BlockRequestHandler::new( &protocol_id, + config.chain_spec.fork_id(), client.clone(), config.network.default_peers_set.in_peers as usize + config.network.default_peers_set.out_peers as usize, @@ -753,6 +754,7 @@ where // Allow both outgoing and incoming requests. let (handler, protocol_config) = StateRequestHandler::new( &protocol_id, + config.chain_spec.fork_id(), client.clone(), config.network.default_peers_set_num_full as usize, ); @@ -762,16 +764,27 @@ where let warp_sync_params = warp_sync.map(|provider| { // Allow both outgoing and incoming requests. - let (handler, protocol_config) = - WarpSyncRequestHandler::new(protocol_id.clone(), provider.clone()); + let (handler, protocol_config) = WarpSyncRequestHandler::new( + protocol_id.clone(), + client + .block_hash(0u32.into()) + .ok() + .flatten() + .expect("Genesis block exists; qed"), + config.chain_spec.fork_id(), + provider.clone(), + ); spawn_handle.spawn("warp-sync-request-handler", Some("networking"), handler.run()); (provider, protocol_config) }); let light_client_request_protocol_config = { // Allow both outgoing and incoming requests. - let (handler, protocol_config) = - LightClientRequestHandler::new(&protocol_id, client.clone()); + let (handler, protocol_config) = LightClientRequestHandler::new( + &protocol_id, + config.chain_spec.fork_id(), + client.clone(), + ); spawn_handle.spawn("light-client-request-handler", Some("networking"), handler.run()); protocol_config }; From d3c4cbfee7d1f91aca9b188bb2d771751babd8fd Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Mon, 1 Aug 2022 20:49:48 +0300 Subject: [PATCH 09/10] Revert "Use sc_network::protocol_name::standard_protocol_name() for BEEFY and GRANDPA" This reverts commit 29aa556ef947e2cfd48264db2a9fc8bc528d7ddb. --- client/beefy/src/lib.rs | 7 +++++-- client/finality-grandpa/src/communication/mod.rs | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index 03d44596cec64..c025ec5686ad2 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -50,7 +50,6 @@ pub use beefy_protocol_name::standard_name as protocol_standard_name; pub(crate) mod beefy_protocol_name { use sc_chain_spec::ChainSpec; - use sc_network::protocol_name::standard_protocol_name; const NAME: &str = "/beefy/1"; /// Old names for the notifications protocol, used for backward compatibility. @@ -63,7 +62,11 @@ pub(crate) mod beefy_protocol_name { genesis_hash: &Hash, chain_spec: &Box, ) -> std::borrow::Cow<'static, str> { - standard_protocol_name(genesis_hash, &chain_spec.fork_id().map(ToOwned::to_owned), NAME) + let chain_prefix = match chain_spec.fork_id() { + 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/mod.rs b/client/finality-grandpa/src/communication/mod.rs index 9b342147804b4..378501cffdd62 100644 --- a/client/finality-grandpa/src/communication/mod.rs +++ b/client/finality-grandpa/src/communication/mod.rs @@ -69,7 +69,6 @@ pub(crate) mod tests; pub mod grandpa_protocol_name { use sc_chain_spec::ChainSpec; - use sc_network::protocol_name::standard_protocol_name; pub(crate) const NAME: &str = "/grandpa/1"; /// Old names for the notifications protocol, used for backward compatibility. @@ -82,7 +81,11 @@ pub mod grandpa_protocol_name { genesis_hash: &Hash, chain_spec: &Box, ) -> std::borrow::Cow<'static, str> { - standard_protocol_name(genesis_hash, &chain_spec.fork_id().map(ToOwned::to_owned), NAME) + let chain_prefix = match chain_spec.fork_id() { + Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id), + None => format!("/{}", hex::encode(genesis_hash)), + }; + format!("{}{}", chain_prefix, NAME).into() } } From 5a3e7479878c194ac0ece99bffe8349f24b5626a Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Mon, 1 Aug 2022 21:19:52 +0300 Subject: [PATCH 10/10] Get rid of `protocol_name` module --- client/network/src/lib.rs | 1 - client/network/src/protocol.rs | 26 ++++---- client/network/src/protocol_name.rs | 97 ----------------------------- client/network/src/transactions.rs | 18 +++--- 4 files changed, 21 insertions(+), 121 deletions(-) delete mode 100644 client/network/src/protocol_name.rs diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs index 10b5ddc47abb4..83bc1075b8bad 100644 --- a/client/network/src/lib.rs +++ b/client/network/src/lib.rs @@ -258,7 +258,6 @@ pub mod bitswap; pub mod config; pub mod error; pub mod network_state; -pub mod protocol_name; pub mod transactions; #[doc(inline)] diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index 396d7b1de9598..a8c38979eb426 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -18,7 +18,6 @@ use crate::{ config, error, - protocol_name::{legacy_protocol_name, standard_protocol_name}, request_responses::RequestFailure, utils::{interval, LruHashSet}, }; @@ -94,9 +93,6 @@ const MAX_BLOCK_ANNOUNCE_SIZE: u64 = 1024 * 1024; // Must be equal to `max(MAX_BLOCK_ANNOUNCE_SIZE, MAX_TRANSACTIONS_SIZE)`. pub(crate) const BLOCK_ANNOUNCES_TRANSACTIONS_SUBSTREAM_SIZE: u64 = 16 * 1024 * 1024; -/// On-the-wire block announces protocol short name (goes after "/{genesis_hash}/{fork_id}"). -const BLOCK_ANNOUNCES_PROTOCOL_SHORT_NAME: &str = "/block-announces/1"; - /// Identifier of the peerset for the block announces protocol. const HARDCODED_PEERSETS_SYNC: sc_peerset::SetId = sc_peerset::SetId::from(0); /// Number of hardcoded peersets (the constants right above). Any set whose identifier is equal or @@ -361,15 +357,17 @@ where sc_peerset::Peerset::from_config(sc_peerset::PeersetConfig { sets }) }; - let block_announces_protocol = standard_protocol_name( - chain.block_hash(0u32.into()).ok().flatten().expect("Genesis block exists; qed"), - fork_id, - BLOCK_ANNOUNCES_PROTOCOL_SHORT_NAME, - ); + let block_announces_protocol = { + let genesis_hash = + chain.block_hash(0u32.into()).ok().flatten().expect("Genesis block exists; qed"); + if let Some(fork_id) = fork_id { + format!("/{}/{}/block-announces/1", hex::encode(genesis_hash), fork_id) + } else { + format!("/{}/block-announces/1", hex::encode(genesis_hash)) + } + }; - let fallback_ba_protocol_names = - iter::once(legacy_protocol_name(&protocol_id, BLOCK_ANNOUNCES_PROTOCOL_SHORT_NAME)) - .collect(); + let legacy_ba_protocol_name = format!("/{}/block-announces/1", protocol_id.as_ref()); let behaviour = { let best_number = info.best_number; @@ -381,8 +379,8 @@ where .encode(); let sync_protocol_config = notifications::ProtocolConfig { - name: block_announces_protocol, - fallback_names: fallback_ba_protocol_names, + name: block_announces_protocol.into(), + fallback_names: iter::once(legacy_ba_protocol_name.into()).collect(), handshake: block_announces_handshake, max_notification_size: MAX_BLOCK_ANNOUNCE_SIZE, }; diff --git a/client/network/src/protocol_name.rs b/client/network/src/protocol_name.rs deleted file mode 100644 index 05bc264ff2e52..0000000000000 --- a/client/network/src/protocol_name.rs +++ /dev/null @@ -1,97 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 - -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -//! On-the-wire protocol name generation helpers. - -use crate::config::ProtocolId; - -/// Standard protocol name on the wire based on `genesis_hash`, `fork_id`, and protocol-specific -/// `short_name`. -/// -/// `short_name` must include the leading slash, e.g.: "/transactions/1". -pub fn standard_protocol_name>( - genesis_hash: Hash, - fork_id: &Option, - short_name: &str, -) -> std::borrow::Cow<'static, str> { - let chain_prefix = match fork_id { - Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id), - None => format!("/{}", hex::encode(genesis_hash)), - }; - format!("{}{}", chain_prefix, short_name).into() -} - -/// Legacy (fallback) protocol name on the wire based on [`ProtocolId`]. -pub fn legacy_protocol_name( - protocol_id: &ProtocolId, - short_name: &str, -) -> std::borrow::Cow<'static, str> { - format!("/{}{}", protocol_id.as_ref(), short_name).into() -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn standard_protocol_name_test() { - const SHORT_NAME: &str = "/protocol/1"; - - // Create protocol name using random genesis hash and no fork id. - let genesis_hash = sp_core::H256::random(); - let fork_id = None; - let expected = format!("/{}/protocol/1", hex::encode(genesis_hash)); - let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); - assert_eq!(protocol_name.to_string(), expected); - - // Create protocol name with fork id. - let fork_id = Some("fork".to_string()); - let expected = format!("/{}/fork/protocol/1", hex::encode(genesis_hash)); - let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); - assert_eq!(protocol_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 fork_id = None; - let expected = - "/354f706177d927ca938ae12658b6d7b96e5808357dd29e973271663bf5c7ddf0/protocol/1"; - let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); - assert_eq!(protocol_name.to_string(), expected.to_string()); - - // Create protocol name using hardcoded genesis hash and fork_id. - // Verify exact representation. - let fork_id = Some("fork".to_string()); - let expected = - "/354f706177d927ca938ae12658b6d7b96e5808357dd29e973271663bf5c7ddf0/fork/protocol/1"; - let protocol_name = standard_protocol_name(genesis_hash, &fork_id, SHORT_NAME); - assert_eq!(protocol_name.to_string(), expected.to_string()); - } - - #[test] - fn legacy_protocol_name_test() { - const SHORT_NAME: &str = "/protocol/1"; - - let protocol_id = ProtocolId::from("dot"); - let expected = "/dot/protocol/1"; - let legacy_name = legacy_protocol_name(&protocol_id, SHORT_NAME); - assert_eq!(legacy_name.to_string(), expected.to_string()); - } -} diff --git a/client/network/src/transactions.rs b/client/network/src/transactions.rs index 646bd2eda8654..342b6a0430272 100644 --- a/client/network/src/transactions.rs +++ b/client/network/src/transactions.rs @@ -30,7 +30,6 @@ use crate::{ config::{self, TransactionImport, TransactionImportFuture, TransactionPool}, error, protocol::message, - protocol_name::{legacy_protocol_name, standard_protocol_name}, service::NetworkService, utils::{interval, LruHashSet}, Event, ExHashT, ObservedRole, @@ -71,9 +70,6 @@ const MAX_TRANSACTIONS_SIZE: u64 = 16 * 1024 * 1024; /// Maximum number of transaction validation request we keep at any moment. const MAX_PENDING_TRANSACTIONS: usize = 8192; -/// Transactions protocol short name (everything after /{genesis_hash}/{fork_id}...). -const PROTOCOL_SHORT_NAME: &str = "/transactions/1"; - mod rep { use sc_peerset::ReputationChange as Rep; /// Reputation change when a peer sends us any transaction. @@ -141,12 +137,16 @@ impl TransactionsHandlerPrototype { genesis_hash: Hash, fork_id: Option, ) -> Self { + let protocol_name = if let Some(fork_id) = fork_id { + format!("/{}/{}/transactions/1", hex::encode(genesis_hash), fork_id) + } else { + format!("/{}/transactions/1", hex::encode(genesis_hash)) + }; + let legacy_protocol_name = format!("/{}/transactions/1", protocol_id.as_ref()); + Self { - protocol_name: standard_protocol_name(genesis_hash, &fork_id, PROTOCOL_SHORT_NAME), - fallback_protocol_names: std::iter::once( - legacy_protocol_name(&protocol_id, PROTOCOL_SHORT_NAME).into(), - ) - .collect(), + protocol_name: protocol_name.into(), + fallback_protocol_names: iter::once(legacy_protocol_name.into()).collect(), } }