From bd4dad824dd9f5606030679bef53d3cc82b657e4 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Mon, 8 Aug 2022 15:00:58 +0300 Subject: [PATCH 01/12] Change request-response protocol names to include genesis hash & fork id --- node/network/bridge/src/network.rs | 15 +++- node/network/bridge/src/tx/mod.rs | 44 ++++++++-- node/network/protocol/Cargo.toml | 1 + .../src/request_response/incoming/mod.rs | 7 +- .../protocol/src/request_response/mod.rs | 84 ++++++++++++++----- node/service/src/lib.rs | 31 +++---- node/service/src/overseer.rs | 15 +++- 7 files changed, 149 insertions(+), 48 deletions(-) diff --git a/node/network/bridge/src/network.rs b/node/network/bridge/src/network.rs index e1310f57414c..012550ef52d6 100644 --- a/node/network/bridge/src/network.rs +++ b/node/network/bridge/src/network.rs @@ -14,7 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -use std::{borrow::Cow, collections::HashSet, sync::Arc}; +use std::{ + borrow::Cow, + collections::{HashMap, HashSet}, + sync::Arc, +}; use async_trait::async_trait; use futures::{prelude::*, stream::BoxStream}; @@ -28,7 +32,7 @@ use sc_network::{ use polkadot_node_network_protocol::{ peer_set::PeerSet, - request_response::{OutgoingRequest, Recipient, Requests}, + request_response::{OutgoingRequest, Protocol, Recipient, Requests}, PeerId, ProtocolVersion, UnifiedReputationChange as Rep, }; use polkadot_primitives::v2::{AuthorityDiscoveryId, Block, Hash}; @@ -97,6 +101,7 @@ pub trait Network: Clone + Send + 'static { &self, authority_discovery: &mut AD, req: Requests, + req_protocols: &HashMap>, if_disconnected: IfDisconnected, ); @@ -153,6 +158,7 @@ impl Network for Arc> { &self, authority_discovery: &mut AD, req: Requests, + req_protocols: &HashMap>, if_disconnected: IfDisconnected, ) { let (protocol, OutgoingRequest { peer, payload, pending_response }) = req.encode_request(); @@ -198,7 +204,10 @@ impl Network for Arc> { NetworkService::start_request( &*self, peer_id, - protocol.into_protocol_name(), + req_protocols + .get(&protocol) + .expect("all `protocol` names are generated via `strum`; qed") + .clone(), payload, pending_response, if_disconnected, diff --git a/node/network/bridge/src/tx/mod.rs b/node/network/bridge/src/tx/mod.rs index d58ccf8fbb95..87673025b799 100644 --- a/node/network/bridge/src/tx/mod.rs +++ b/node/network/bridge/src/tx/mod.rs @@ -15,9 +15,13 @@ // along with Polkadot. If not, see . //! The Network Bridge Subsystem - handles _outgoing_ messages, from subsystem to the network. +use std::borrow::Cow; + use super::*; -use polkadot_node_network_protocol::{peer_set::PeerSet, v1 as protocol_v1, PeerId, Versioned}; +use polkadot_node_network_protocol::{ + peer_set::PeerSet, request_response::Protocol, v1 as protocol_v1, PeerId, Versioned, +}; use polkadot_node_subsystem::{ errors::SubsystemError, messages::NetworkBridgeTxMessage, overseer, FromOrchestra, @@ -50,6 +54,7 @@ pub struct NetworkBridgeTx { network_service: N, authority_discovery_service: AD, metrics: Metrics, + requests_protocols: HashMap>, } impl NetworkBridgeTx { @@ -57,8 +62,13 @@ impl NetworkBridgeTx { /// /// This assumes that the network service has had the notifications protocol for the network /// bridge already registered. See [`peers_sets_info`](peers_sets_info). - pub fn new(network_service: N, authority_discovery_service: AD, metrics: Metrics) -> Self { - Self { network_service, authority_discovery_service, metrics } + pub fn new( + network_service: N, + authority_discovery_service: AD, + metrics: Metrics, + requests_protocols: HashMap>, + ) -> Self { + Self { network_service, authority_discovery_service, metrics, requests_protocols } } } @@ -82,6 +92,7 @@ async fn handle_subsystem_messages( mut network_service: N, mut authority_discovery_service: AD, metrics: Metrics, + requests_protocols: &HashMap>, ) -> Result<(), Error> where N: Network, @@ -102,6 +113,7 @@ where authority_discovery_service.clone(), msg, &metrics, + requests_protocols, ) .await; }, @@ -117,6 +129,7 @@ async fn handle_incoming_subsystem_communication( mut authority_discovery_service: AD, msg: NetworkBridgeTxMessage, metrics: &Metrics, + requests_protocols: &HashMap>, ) -> (N, AD) where N: Network, @@ -218,7 +231,12 @@ where for req in reqs { network_service - .start_request(&mut authority_discovery_service, req, if_disconnected) + .start_request( + &mut authority_discovery_service, + req, + requests_protocols, + if_disconnected, + ) .await; } }, @@ -275,9 +293,21 @@ where N: Network, AD: validator_discovery::AuthorityDiscovery + Clone + Sync, { - let NetworkBridgeTx { network_service, authority_discovery_service, metrics } = bridge; - - handle_subsystem_messages(ctx, network_service, authority_discovery_service, metrics).await?; + let NetworkBridgeTx { + network_service, + authority_discovery_service, + metrics, + requests_protocols, + } = bridge; + + handle_subsystem_messages( + ctx, + network_service, + authority_discovery_service, + metrics, + &requests_protocols, + ) + .await?; Ok(()) } diff --git a/node/network/protocol/Cargo.toml b/node/network/protocol/Cargo.toml index 7b26a03e3d44..8b863089cc91 100644 --- a/node/network/protocol/Cargo.toml +++ b/node/network/protocol/Cargo.toml @@ -7,6 +7,7 @@ description = "Primitives types for the Node-side" [dependencies] async-trait = "0.1.53" +hex = "0.4.3" polkadot-primitives = { path = "../../../primitives" } polkadot-node-primitives = { path = "../../primitives" } polkadot-node-jaeger = { path = "../../jaeger" } diff --git a/node/network/protocol/src/request_response/incoming/mod.rs b/node/network/protocol/src/request_response/incoming/mod.rs index 309ca32b0de4..38fcddb5c3b4 100644 --- a/node/network/protocol/src/request_response/incoming/mod.rs +++ b/node/network/protocol/src/request_response/incoming/mod.rs @@ -55,8 +55,11 @@ where /// /// This Register that config with substrate networking and receive incoming requests via the /// returned `IncomingRequestReceiver`. - pub fn get_config_receiver() -> (IncomingRequestReceiver, RequestResponseConfig) { - let (raw, cfg) = Req::PROTOCOL.get_config(); + pub fn get_config_receiver>( + genesis_hash: &Hash, + fork_id: &Option, + ) -> (IncomingRequestReceiver, RequestResponseConfig) { + let (raw, cfg) = Req::PROTOCOL.get_config(genesis_hash, fork_id); (IncomingRequestReceiver { raw, phantom: PhantomData {} }, cfg) } diff --git a/node/network/protocol/src/request_response/mod.rs b/node/network/protocol/src/request_response/mod.rs index b434c152b895..045e4253e3d6 100644 --- a/node/network/protocol/src/request_response/mod.rs +++ b/node/network/protocol/src/request_response/mod.rs @@ -32,11 +32,11 @@ //! //! Versioned (v1 module): The actual requests and responses as sent over the network. -use std::{borrow::Cow, time::Duration, u64}; +use std::{borrow::Cow, collections::HashMap, time::Duration, u64}; use futures::channel::mpsc; use polkadot_primitives::v2::{MAX_CODE_SIZE, MAX_POV_SIZE}; -use strum::EnumIter; +use strum::{EnumIter, IntoEnumIterator}; pub use sc_network::{config as network, config::RequestResponseConfig}; @@ -126,13 +126,18 @@ impl Protocol { /// /// Returns a receiver for messages received on this protocol and the requested /// `ProtocolConfig`. - pub fn get_config(self) -> (mpsc::Receiver, RequestResponseConfig) { - let p_name = self.into_protocol_name(); + pub fn get_config>( + self, + genesis_hash: &Hash, + fork_id: &Option, + ) -> (mpsc::Receiver, RequestResponseConfig) { + let name = self.protocol_name(genesis_hash, fork_id); + let fallback_names = self.fallback_names(); let (tx, rx) = mpsc::channel(self.get_channel_size()); let cfg = match self { Protocol::ChunkFetchingV1 => RequestResponseConfig { - name: p_name, - fallback_names: Vec::new(), + name, + fallback_names, max_request_size: 1_000, max_response_size: POV_RESPONSE_SIZE as u64 * 3, // We are connected to all validators: @@ -140,8 +145,8 @@ impl Protocol { inbound_queue: Some(tx), }, Protocol::CollationFetchingV1 => RequestResponseConfig { - name: p_name, - fallback_names: Vec::new(), + name, + fallback_names, max_request_size: 1_000, max_response_size: POV_RESPONSE_SIZE, // Taken from initial implementation in collator protocol: @@ -149,16 +154,16 @@ impl Protocol { inbound_queue: Some(tx), }, Protocol::PoVFetchingV1 => RequestResponseConfig { - name: p_name, - fallback_names: Vec::new(), + name, + fallback_names, max_request_size: 1_000, max_response_size: POV_RESPONSE_SIZE, request_timeout: POV_REQUEST_TIMEOUT_CONNECTED, inbound_queue: Some(tx), }, Protocol::AvailableDataFetchingV1 => RequestResponseConfig { - name: p_name, - fallback_names: Vec::new(), + name, + fallback_names, max_request_size: 1_000, // Available data size is dominated by the PoV size. max_response_size: POV_RESPONSE_SIZE, @@ -166,8 +171,8 @@ impl Protocol { inbound_queue: Some(tx), }, Protocol::StatementFetchingV1 => RequestResponseConfig { - name: p_name, - fallback_names: Vec::new(), + name, + fallback_names, max_request_size: 1_000, // Available data size is dominated code size. max_response_size: STATEMENT_RESPONSE_SIZE, @@ -184,8 +189,8 @@ impl Protocol { inbound_queue: Some(tx), }, Protocol::DisputeSendingV1 => RequestResponseConfig { - name: p_name, - fallback_names: Vec::new(), + name, + fallback_names, max_request_size: 1_000, /// Responses are just confirmation, in essence not even a bit. So 100 seems /// plenty. @@ -243,13 +248,13 @@ impl Protocol { } } - /// Get the protocol name of this protocol, as understood by substrate networking. - pub fn into_protocol_name(self) -> Cow<'static, str> { - self.get_protocol_name_static().into() + /// Fallback protocol names of this protocol, as understood by substrate networking. + pub fn fallback_names(self) -> Vec> { + std::iter::once(self.legacy_name().into()).collect() } - /// Get the protocol name associated with each peer set as static str. - pub const fn get_protocol_name_static(self) -> &'static str { + /// Legacy protocol name associated with each peer set. + pub const fn legacy_name(self) -> &'static str { match self { Protocol::ChunkFetchingV1 => "/polkadot/req_chunk/1", Protocol::CollationFetchingV1 => "/polkadot/req_collation/1", @@ -259,6 +264,43 @@ impl Protocol { Protocol::DisputeSendingV1 => "/polkadot/send_dispute/1", } } + + /// Protocol name of this protocol, as understood by substrate networking. + pub fn protocol_name>( + self, + genesis_hash: &Hash, + fork_id: &Option, + ) -> Cow<'static, str> { + let prefix = if let Some(fork_id) = fork_id { + format!("/{}/{}", hex::encode(genesis_hash), fork_id) + } else { + format!("/{}", hex::encode(genesis_hash)) + }; + + let short_name = match self { + Protocol::ChunkFetchingV1 => "/req_chunk/1", + Protocol::CollationFetchingV1 => "/req_collation/1", + Protocol::PoVFetchingV1 => "/req_pov/1", + Protocol::AvailableDataFetchingV1 => "/req_available_data/1", + Protocol::StatementFetchingV1 => "/req_statement/1", + Protocol::DisputeSendingV1 => "/send_dispute/1", + }; + + format!("{}{}", prefix, short_name).into() + } + + /// All protocol names as understood by substrate networking for specific `genesis_hash` + /// and `fork_id`, represented by a HashMap. + pub fn protocol_names>( + genesis_hash: &Hash, + fork_id: &Option, + ) -> HashMap> { + let mut p_names = HashMap::new(); + for protocol in Protocol::iter() { + p_names.insert(protocol, protocol.protocol_name(genesis_hash, fork_id)); + } + p_names + } } /// Common properties of any `Request`. diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index c39be9d85701..0fbd64be428a 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -831,22 +831,19 @@ where let shared_voter_state = rpc_setup; let auth_disc_publish_non_global_ips = config.network.allow_non_globals_in_dht; + let genesis_hash = client.block_hash(0).ok().flatten().expect("Genesis block exists; qed"); + // Note: GrandPa is pushed before the Polkadot-specific protocols. This doesn't change // anything in terms of behaviour, but makes the logs more consistent with the other // Substrate nodes. - let grandpa_protocol_name = grandpa::protocol_standard_name( - &client.block_hash(0).ok().flatten().expect("Genesis block exists; qed"), - &config.chain_spec, - ); + let grandpa_protocol_name = grandpa::protocol_standard_name(&genesis_hash, &config.chain_spec); config .network .extra_sets .push(grandpa::grandpa_peers_set_config(grandpa_protocol_name.clone())); - let beefy_protocol_name = beefy_gadget::protocol_standard_name( - &client.block_hash(0).ok().flatten().expect("Genesis block exists; qed"), - &config.chain_spec, - ); + let beefy_protocol_name = + beefy_gadget::protocol_standard_name(&genesis_hash, &config.chain_spec); if enable_beefy { config .network @@ -860,17 +857,22 @@ where config.network.extra_sets.extend(peer_sets_info(is_authority)); } - let (pov_req_receiver, cfg) = IncomingRequest::get_config_receiver(); + let fork_id = config.chain_spec.fork_id().map(ToOwned::to_owned); + + let (pov_req_receiver, cfg) = IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); config.network.request_response_protocols.push(cfg); - let (chunk_req_receiver, cfg) = IncomingRequest::get_config_receiver(); + let (chunk_req_receiver, cfg) = IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); config.network.request_response_protocols.push(cfg); - let (collation_req_receiver, cfg) = IncomingRequest::get_config_receiver(); + let (collation_req_receiver, cfg) = + IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); config.network.request_response_protocols.push(cfg); - let (available_data_req_receiver, cfg) = IncomingRequest::get_config_receiver(); + let (available_data_req_receiver, cfg) = + IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); config.network.request_response_protocols.push(cfg); - let (statement_req_receiver, cfg) = IncomingRequest::get_config_receiver(); + let (statement_req_receiver, cfg) = + IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); config.network.request_response_protocols.push(cfg); - let (dispute_req_receiver, cfg) = IncomingRequest::get_config_receiver(); + let (dispute_req_receiver, cfg) = IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); config.network.request_response_protocols.push(cfg); let grandpa_hard_forks = if config.chain_spec.is_kusama() { @@ -1060,6 +1062,7 @@ where dispute_coordinator_config, pvf_checker_enabled, overseer_message_channel_capacity_override, + fork_id, }, ) .map_err(|e| { diff --git a/node/service/src/overseer.rs b/node/service/src/overseer.rs index efc36de15423..cecab314d542 100644 --- a/node/service/src/overseer.rs +++ b/node/service/src/overseer.rs @@ -24,7 +24,9 @@ use polkadot_node_core_av_store::Config as AvailabilityConfig; use polkadot_node_core_candidate_validation::Config as CandidateValidationConfig; use polkadot_node_core_chain_selection::Config as ChainSelectionConfig; use polkadot_node_core_dispute_coordinator::Config as DisputeCoordinatorConfig; -use polkadot_node_network_protocol::request_response::{v1 as request_v1, IncomingRequestReceiver}; +use polkadot_node_network_protocol::request_response::{ + v1 as request_v1, IncomingRequestReceiver, Protocol as RequestsProtocol, +}; #[cfg(any(feature = "malus", test))] pub use polkadot_overseer::{ dummy::{dummy_overseer_builder, DummySubsystem}, @@ -117,6 +119,9 @@ where pub pvf_checker_enabled: bool, /// Overseer channel capacity override. pub overseer_message_channel_capacity_override: Option, + /// Fork ID from the client spec to only talk to "our" nodes if multiple chains + /// have the same genesis hash. + pub fork_id: Option, } /// Obtain a prepared `OverseerBuilder`, that is initialized @@ -145,6 +150,7 @@ pub fn prepared_overseer_builder<'a, Spawner, RuntimeClient>( dispute_coordinator_config, pvf_checker_enabled, overseer_message_channel_capacity_override, + fork_id, }: OverseerGenArgs<'a, Spawner, RuntimeClient>, ) -> Result< InitializedOverseerBuilder< @@ -193,11 +199,18 @@ where let spawner = SpawnGlue(spawner); let network_bridge_metrics: NetworkBridgeMetrics = Metrics::register(registry)?; + + let requests_protocols = RequestsProtocol::protocol_names( + &runtime_client.hash(0).ok().flatten().expect("Genesis block exists; qed"), + &fork_id, + ); + let builder = Overseer::builder() .network_bridge_tx(NetworkBridgeTxSubsystem::new( network_service.clone(), authority_discovery_service.clone(), network_bridge_metrics.clone(), + requests_protocols, )) .network_bridge_rx(NetworkBridgeRxSubsystem::new( network_service.clone(), From 04b3f2c2a5644ac6a6e6de7502d73de4a36d514d Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Mon, 8 Aug 2022 16:48:51 +0300 Subject: [PATCH 02/12] Change request-response protocol names: fix tests --- .../availability-distribution/src/tests/mod.rs | 9 ++++++--- node/network/availability-recovery/src/tests.rs | 11 ++++++++--- node/network/bridge/src/rx/tests.rs | 3 ++- node/network/bridge/src/tx/tests.rs | 8 ++++++-- node/network/bridge/src/validator_discovery.rs | 6 +++++- .../collator-protocol/src/collator_side/tests.rs | 5 ++++- .../network/dispute-distribution/src/tests/mod.rs | 3 ++- node/network/statement-distribution/src/tests.rs | 15 ++++++++++----- 8 files changed, 43 insertions(+), 17 deletions(-) diff --git a/node/network/availability-distribution/src/tests/mod.rs b/node/network/availability-distribution/src/tests/mod.rs index fd5d0dafaf1f..08e17426d481 100644 --- a/node/network/availability-distribution/src/tests/mod.rs +++ b/node/network/availability-distribution/src/tests/mod.rs @@ -19,7 +19,7 @@ use std::collections::HashSet; use futures::{executor, future, Future}; use polkadot_node_network_protocol::request_response::IncomingRequest; -use polkadot_primitives::v2::CoreState; +use polkadot_primitives::v2::{CoreState, Hash}; use sp_keystore::SyncCryptoStorePtr; use polkadot_node_subsystem_test_helpers as test_helpers; @@ -41,9 +41,12 @@ fn test_harness>( let pool = sp_core::testing::TaskExecutor::new(); let (context, virtual_overseer) = test_helpers::make_subsystem_context(pool.clone()); + let genesis_hash = Hash::repeat_byte(0xff); - let (pov_req_receiver, pov_req_cfg) = IncomingRequest::get_config_receiver(); - let (chunk_req_receiver, chunk_req_cfg) = IncomingRequest::get_config_receiver(); + let (pov_req_receiver, pov_req_cfg) = + IncomingRequest::get_config_receiver(&genesis_hash, &None); + let (chunk_req_receiver, chunk_req_cfg) = + IncomingRequest::get_config_receiver(&genesis_hash, &None); let subsystem = AvailabilityDistributionSubsystem::new( keystore, IncomingRequestReceivers { pov_req_receiver, chunk_req_receiver }, diff --git a/node/network/availability-recovery/src/tests.rs b/node/network/availability-recovery/src/tests.rs index 3c16157f4882..36710eeec6aa 100644 --- a/node/network/availability-recovery/src/tests.rs +++ b/node/network/availability-recovery/src/tests.rs @@ -36,11 +36,14 @@ use polkadot_node_subsystem::{ }; use polkadot_node_subsystem_test_helpers::{make_subsystem_context, TestSubsystemContextHandle}; use polkadot_node_subsystem_util::TimeoutExt; -use polkadot_primitives::v2::{AuthorityDiscoveryId, HeadData, PersistedValidationData}; +use polkadot_primitives::v2::{AuthorityDiscoveryId, Hash, HeadData, PersistedValidationData}; use polkadot_primitives_test_helpers::{dummy_candidate_receipt, dummy_hash}; type VirtualOverseer = TestSubsystemContextHandle; +// Deterministic genesis hash for protocol names +const GENESIS_HASH: Hash = Hash::repeat_byte(0xff); + fn test_harness_fast_path>( test: impl FnOnce(VirtualOverseer, RequestResponseConfig) -> T, ) { @@ -53,7 +56,8 @@ fn test_harness_fast_path>, _: IfDisconnected, ) { } diff --git a/node/network/bridge/src/tx/tests.rs b/node/network/bridge/src/tx/tests.rs index 63d9730e6599..c08d840e5cc6 100644 --- a/node/network/bridge/src/tx/tests.rs +++ b/node/network/bridge/src/tx/tests.rs @@ -30,7 +30,7 @@ use polkadot_node_network_protocol::{ use polkadot_node_subsystem::{FromOrchestra, OverseerSignal}; use polkadot_node_subsystem_test_helpers::TestSubsystemContextHandle; use polkadot_node_subsystem_util::metered; -use polkadot_primitives::v2::AuthorityDiscoveryId; +use polkadot_primitives::v2::{AuthorityDiscoveryId, Hash}; use polkadot_primitives_test_helpers::dummy_collator_signature; use sc_network::Multiaddr; use sp_keyring::Sr25519Keyring; @@ -104,6 +104,7 @@ impl Network for TestNetwork { &self, _: &mut AD, _: Requests, + _: &HashMap>, _: IfDisconnected, ) { } @@ -182,7 +183,10 @@ fn test_harness>(test: impl FnOnce(TestHarne let (context, virtual_overseer) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let bridge_out = NetworkBridgeTx::new(network, discovery, Metrics(None)); + let genesis_hash = Hash::repeat_byte(0xff); + let protocol_names = Protocol::protocol_names(&genesis_hash, &None); + + let bridge_out = NetworkBridgeTx::new(network, discovery, Metrics(None), protocol_names); let network_bridge_out_fut = run_network_out(bridge_out, context) .map_err(|e| panic!("bridge-out subsystem execution failed {:?}", e)) diff --git a/node/network/bridge/src/validator_discovery.rs b/node/network/bridge/src/validator_discovery.rs index eb9bb954e7a1..30c60c6d9e75 100644 --- a/node/network/bridge/src/validator_discovery.rs +++ b/node/network/bridge/src/validator_discovery.rs @@ -162,7 +162,10 @@ mod tests { use async_trait::async_trait; use futures::stream::BoxStream; - use polkadot_node_network_protocol::{request_response::outgoing::Requests, PeerId}; + use polkadot_node_network_protocol::{ + request_response::{outgoing::Requests, Protocol}, + PeerId, + }; use sc_network::{Event as NetworkEvent, IfDisconnected}; use sp_keyring::Sr25519Keyring; use std::{ @@ -236,6 +239,7 @@ mod tests { &self, _: &mut AD, _: Requests, + _: &HashMap>, _: IfDisconnected, ) { } diff --git a/node/network/collator-protocol/src/collator_side/tests.rs b/node/network/collator-protocol/src/collator_side/tests.rs index 4d95b7c807e2..7377685c7e64 100644 --- a/node/network/collator-protocol/src/collator_side/tests.rs +++ b/node/network/collator-protocol/src/collator_side/tests.rs @@ -201,7 +201,10 @@ fn test_harness>( let (context, virtual_overseer) = test_helpers::make_subsystem_context(pool.clone()); - let (collation_req_receiver, req_cfg) = IncomingRequest::get_config_receiver(); + let genesis_hash = Hash::repeat_byte(0xff); + + let (collation_req_receiver, req_cfg) = + IncomingRequest::get_config_receiver(&genesis_hash, &None); let subsystem = async { run(context, local_peer_id, collator_pair, collation_req_receiver, Default::default()) .await diff --git a/node/network/dispute-distribution/src/tests/mod.rs b/node/network/dispute-distribution/src/tests/mod.rs index 9c843f3e786b..7abc65dbac46 100644 --- a/node/network/dispute-distribution/src/tests/mod.rs +++ b/node/network/dispute-distribution/src/tests/mod.rs @@ -723,7 +723,8 @@ where sp_tracing::try_init_simple(); let keystore = make_ferdie_keystore(); - let (req_receiver, req_cfg) = IncomingRequest::get_config_receiver(); + let genesis_hash = Hash::repeat_byte(0xff); + let (req_receiver, req_cfg) = IncomingRequest::get_config_receiver(&genesis_hash, &None); let subsystem = DisputeDistributionSubsystem::new( keystore, req_receiver, diff --git a/node/network/statement-distribution/src/tests.rs b/node/network/statement-distribution/src/tests.rs index 49a6e211bbd6..d598421c95da 100644 --- a/node/network/statement-distribution/src/tests.rs +++ b/node/network/statement-distribution/src/tests.rs @@ -44,6 +44,9 @@ use sp_keyring::Sr25519Keyring; use sp_keystore::{CryptoStore, SyncCryptoStore, SyncCryptoStorePtr}; use std::{iter::FromIterator as _, sync::Arc, time::Duration}; +// Some deterministic genesis hash for protocol names +const GENESIS_HASH: Hash = Hash::repeat_byte(0xff); + #[test] fn active_head_accepts_only_2_seconded_per_validator() { let validators = vec![ @@ -724,7 +727,7 @@ fn receiving_from_one_sends_to_another_and_to_candidate_backing() { let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(); + let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, &None); let bg = async move { let s = StatementDistributionSubsystem::new( @@ -917,7 +920,8 @@ fn receiving_large_statement_from_one_sends_to_another_and_to_candidate_backing( let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let (statement_req_receiver, mut req_cfg) = IncomingRequest::get_config_receiver(); + let (statement_req_receiver, mut req_cfg) = + IncomingRequest::get_config_receiver(&GENESIS_HASH, &None); let bg = async move { let s = StatementDistributionSubsystem::new( @@ -1429,7 +1433,8 @@ fn share_prioritizes_backing_group() { let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let (statement_req_receiver, mut req_cfg) = IncomingRequest::get_config_receiver(); + let (statement_req_receiver, mut req_cfg) = + IncomingRequest::get_config_receiver(&GENESIS_HASH, &None); let bg = async move { let s = StatementDistributionSubsystem::new( @@ -1724,7 +1729,7 @@ fn peer_cant_flood_with_large_statements() { let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(); + let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, &None); let bg = async move { let s = StatementDistributionSubsystem::new( make_ferdie_keystore(), @@ -1928,7 +1933,7 @@ fn handle_multiple_seconded_statements() { let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(); + let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, &None); let virtual_overseer_fut = async move { let s = StatementDistributionSubsystem::new( From 15e6ff30045315bea16711a9a675741e03cff72f Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Mon, 8 Aug 2022 16:57:54 +0300 Subject: [PATCH 03/12] Change request-response protocol names: fix docs --- .../src/node/disputes/dispute-distribution.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/roadmap/implementers-guide/src/node/disputes/dispute-distribution.md b/roadmap/implementers-guide/src/node/disputes/dispute-distribution.md index eb571420fb78..579a1ce8f143 100644 --- a/roadmap/implementers-guide/src/node/disputes/dispute-distribution.md +++ b/roadmap/implementers-guide/src/node/disputes/dispute-distribution.md @@ -30,7 +30,7 @@ This design should result in a protocol that is: #### Disputes -Protocol: `"/polkadot/send_dispute/1"` +Protocol: `"///send_dispute/1"` Request: @@ -86,7 +86,7 @@ enum DisputeResponse { #### Vote Recovery -Protocol: `"/polkadot/req_votes/1"` +Protocol: `"///req_votes/1"` ```rust struct IHaveVotesRequest { From a18e9ca7b202953a5bfe7f9c1924e8993b7ad2d7 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Tue, 9 Aug 2022 11:45:38 +0300 Subject: [PATCH 04/12] Change request-response protocol names: commit Cargo.lock --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index 3d7609e2dea4..9e0dc3f6cc17 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6667,6 +6667,7 @@ dependencies = [ "derive_more", "fatality", "futures", + "hex", "parity-scale-codec", "polkadot-node-jaeger", "polkadot-node-primitives", From f6aa2c1c8b4c91d7c67ce2b5490e876a0f86bed1 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Tue, 9 Aug 2022 13:45:54 +0300 Subject: [PATCH 05/12] Change request-response protocol names: fix spelling --- node/network/protocol/src/request_response/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/network/protocol/src/request_response/mod.rs b/node/network/protocol/src/request_response/mod.rs index 045e4253e3d6..5999a8ca4587 100644 --- a/node/network/protocol/src/request_response/mod.rs +++ b/node/network/protocol/src/request_response/mod.rs @@ -290,7 +290,7 @@ impl Protocol { } /// All protocol names as understood by substrate networking for specific `genesis_hash` - /// and `fork_id`, represented by a HashMap. + /// and `fork_id`, represented by a hash map. pub fn protocol_names>( genesis_hash: &Hash, fork_id: &Option, From fa8c22534a23ae7afdc5c8df929d5da80ffd58aa Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Wed, 10 Aug 2022 20:47:01 +0300 Subject: [PATCH 06/12] Implement PR review suggestions --- .../src/tests/mod.rs | 5 +- .../availability-recovery/src/tests.rs | 4 +- node/network/bridge/src/network.rs | 17 ++---- node/network/bridge/src/rx/tests.rs | 7 +-- node/network/bridge/src/tx/mod.rs | 22 ++++---- node/network/bridge/src/tx/tests.rs | 7 +-- .../network/bridge/src/validator_discovery.rs | 4 +- .../src/collator_side/tests.rs | 2 +- .../dispute-distribution/src/tests/mod.rs | 2 +- .../src/request_response/incoming/mod.rs | 5 +- .../protocol/src/request_response/mod.rs | 54 +++++++++---------- .../statement-distribution/src/tests.rs | 10 ++-- node/service/src/lib.rs | 15 +++--- node/service/src/overseer.rs | 10 ++-- 14 files changed, 79 insertions(+), 85 deletions(-) diff --git a/node/network/availability-distribution/src/tests/mod.rs b/node/network/availability-distribution/src/tests/mod.rs index 08e17426d481..77b85f9a565d 100644 --- a/node/network/availability-distribution/src/tests/mod.rs +++ b/node/network/availability-distribution/src/tests/mod.rs @@ -43,10 +43,9 @@ fn test_harness>( let (context, virtual_overseer) = test_helpers::make_subsystem_context(pool.clone()); let genesis_hash = Hash::repeat_byte(0xff); - let (pov_req_receiver, pov_req_cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, &None); + let (pov_req_receiver, pov_req_cfg) = IncomingRequest::get_config_receiver(&genesis_hash, None); let (chunk_req_receiver, chunk_req_cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, &None); + IncomingRequest::get_config_receiver(&genesis_hash, None); let subsystem = AvailabilityDistributionSubsystem::new( keystore, IncomingRequestReceivers { pov_req_receiver, chunk_req_receiver }, diff --git a/node/network/availability-recovery/src/tests.rs b/node/network/availability-recovery/src/tests.rs index 36710eeec6aa..ae8b12ae6e9d 100644 --- a/node/network/availability-recovery/src/tests.rs +++ b/node/network/availability-recovery/src/tests.rs @@ -57,7 +57,7 @@ fn test_harness_fast_path. -use std::{ - borrow::Cow, - collections::{HashMap, HashSet}, - sync::Arc, -}; +use std::{borrow::Cow, collections::HashSet, sync::Arc}; use async_trait::async_trait; use futures::{prelude::*, stream::BoxStream}; @@ -32,7 +28,7 @@ use sc_network::{ use polkadot_node_network_protocol::{ peer_set::PeerSet, - request_response::{OutgoingRequest, Protocol, Recipient, Requests}, + request_response::{OutgoingRequest, Recipient, ReqProtocolNames, Requests}, PeerId, ProtocolVersion, UnifiedReputationChange as Rep, }; use polkadot_primitives::v2::{AuthorityDiscoveryId, Block, Hash}; @@ -101,7 +97,7 @@ pub trait Network: Clone + Send + 'static { &self, authority_discovery: &mut AD, req: Requests, - req_protocols: &HashMap>, + req_protocol_names: &ReqProtocolNames, if_disconnected: IfDisconnected, ); @@ -158,7 +154,7 @@ impl Network for Arc> { &self, authority_discovery: &mut AD, req: Requests, - req_protocols: &HashMap>, + req_protocol_names: &ReqProtocolNames, if_disconnected: IfDisconnected, ) { let (protocol, OutgoingRequest { peer, payload, pending_response }) = req.encode_request(); @@ -204,10 +200,7 @@ impl Network for Arc> { NetworkService::start_request( &*self, peer_id, - req_protocols - .get(&protocol) - .expect("all `protocol` names are generated via `strum`; qed") - .clone(), + req_protocol_names.get_name(protocol), payload, pending_response, if_disconnected, diff --git a/node/network/bridge/src/rx/tests.rs b/node/network/bridge/src/rx/tests.rs index 7594791a7c7b..09329f9bad2d 100644 --- a/node/network/bridge/src/rx/tests.rs +++ b/node/network/bridge/src/rx/tests.rs @@ -16,7 +16,7 @@ use super::*; use futures::{channel::oneshot, executor, stream::BoxStream}; -use polkadot_node_network_protocol::{self as net_protocol, request_response::Protocol, OurView}; +use polkadot_node_network_protocol::{self as net_protocol, OurView}; use polkadot_node_subsystem::{messages::NetworkBridgeEvent, ActivatedLeaf}; use assert_matches::assert_matches; @@ -31,7 +31,8 @@ use std::{ use sc_network::{Event as NetworkEvent, IfDisconnected}; use polkadot_node_network_protocol::{ - request_response::outgoing::Requests, view, ObservedRole, Versioned, + request_response::{outgoing::Requests, ReqProtocolNames}, + view, ObservedRole, Versioned, }; use polkadot_node_subsystem::{ jaeger, @@ -117,7 +118,7 @@ impl Network for TestNetwork { &self, _: &mut AD, _: Requests, - _: &HashMap>, + _: &ReqProtocolNames, _: IfDisconnected, ) { } diff --git a/node/network/bridge/src/tx/mod.rs b/node/network/bridge/src/tx/mod.rs index 87673025b799..ac34cf315cec 100644 --- a/node/network/bridge/src/tx/mod.rs +++ b/node/network/bridge/src/tx/mod.rs @@ -15,12 +15,10 @@ // along with Polkadot. If not, see . //! The Network Bridge Subsystem - handles _outgoing_ messages, from subsystem to the network. -use std::borrow::Cow; - use super::*; use polkadot_node_network_protocol::{ - peer_set::PeerSet, request_response::Protocol, v1 as protocol_v1, PeerId, Versioned, + peer_set::PeerSet, request_response::ReqProtocolNames, v1 as protocol_v1, PeerId, Versioned, }; use polkadot_node_subsystem::{ @@ -54,7 +52,7 @@ pub struct NetworkBridgeTx { network_service: N, authority_discovery_service: AD, metrics: Metrics, - requests_protocols: HashMap>, + req_protocol_names: ReqProtocolNames, } impl NetworkBridgeTx { @@ -66,9 +64,9 @@ impl NetworkBridgeTx { network_service: N, authority_discovery_service: AD, metrics: Metrics, - requests_protocols: HashMap>, + req_protocol_names: ReqProtocolNames, ) -> Self { - Self { network_service, authority_discovery_service, metrics, requests_protocols } + Self { network_service, authority_discovery_service, metrics, req_protocol_names } } } @@ -92,7 +90,7 @@ async fn handle_subsystem_messages( mut network_service: N, mut authority_discovery_service: AD, metrics: Metrics, - requests_protocols: &HashMap>, + req_protocol_names: ReqProtocolNames, ) -> Result<(), Error> where N: Network, @@ -113,7 +111,7 @@ where authority_discovery_service.clone(), msg, &metrics, - requests_protocols, + &req_protocol_names, ) .await; }, @@ -129,7 +127,7 @@ async fn handle_incoming_subsystem_communication( mut authority_discovery_service: AD, msg: NetworkBridgeTxMessage, metrics: &Metrics, - requests_protocols: &HashMap>, + req_protocol_names: &ReqProtocolNames, ) -> (N, AD) where N: Network, @@ -234,7 +232,7 @@ where .start_request( &mut authority_discovery_service, req, - requests_protocols, + req_protocol_names, if_disconnected, ) .await; @@ -297,7 +295,7 @@ where network_service, authority_discovery_service, metrics, - requests_protocols, + req_protocol_names, } = bridge; handle_subsystem_messages( @@ -305,7 +303,7 @@ where network_service, authority_discovery_service, metrics, - &requests_protocols, + req_protocol_names, ) .await?; diff --git a/node/network/bridge/src/tx/tests.rs b/node/network/bridge/src/tx/tests.rs index c08d840e5cc6..d5b6d3ca67ab 100644 --- a/node/network/bridge/src/tx/tests.rs +++ b/node/network/bridge/src/tx/tests.rs @@ -25,7 +25,8 @@ use std::{borrow::Cow, collections::HashSet}; use sc_network::{Event as NetworkEvent, IfDisconnected}; use polkadot_node_network_protocol::{ - request_response::outgoing::Requests, ObservedRole, Versioned, + request_response::{outgoing::Requests, ReqProtocolNames}, + ObservedRole, Versioned, }; use polkadot_node_subsystem::{FromOrchestra, OverseerSignal}; use polkadot_node_subsystem_test_helpers::TestSubsystemContextHandle; @@ -104,7 +105,7 @@ impl Network for TestNetwork { &self, _: &mut AD, _: Requests, - _: &HashMap>, + _: &ReqProtocolNames, _: IfDisconnected, ) { } @@ -184,7 +185,7 @@ fn test_harness>(test: impl FnOnce(TestHarne polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); let genesis_hash = Hash::repeat_byte(0xff); - let protocol_names = Protocol::protocol_names(&genesis_hash, &None); + let protocol_names = ReqProtocolNames::new(genesis_hash, None); let bridge_out = NetworkBridgeTx::new(network, discovery, Metrics(None), protocol_names); diff --git a/node/network/bridge/src/validator_discovery.rs b/node/network/bridge/src/validator_discovery.rs index 30c60c6d9e75..9c90200aa06a 100644 --- a/node/network/bridge/src/validator_discovery.rs +++ b/node/network/bridge/src/validator_discovery.rs @@ -163,7 +163,7 @@ mod tests { use async_trait::async_trait; use futures::stream::BoxStream; use polkadot_node_network_protocol::{ - request_response::{outgoing::Requests, Protocol}, + request_response::{outgoing::Requests, ReqProtocolNames}, PeerId, }; use sc_network::{Event as NetworkEvent, IfDisconnected}; @@ -239,7 +239,7 @@ mod tests { &self, _: &mut AD, _: Requests, - _: &HashMap>, + _: &ReqProtocolNames, _: IfDisconnected, ) { } diff --git a/node/network/collator-protocol/src/collator_side/tests.rs b/node/network/collator-protocol/src/collator_side/tests.rs index 7377685c7e64..5a398462014e 100644 --- a/node/network/collator-protocol/src/collator_side/tests.rs +++ b/node/network/collator-protocol/src/collator_side/tests.rs @@ -204,7 +204,7 @@ fn test_harness>( let genesis_hash = Hash::repeat_byte(0xff); let (collation_req_receiver, req_cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, &None); + IncomingRequest::get_config_receiver(&genesis_hash, None); let subsystem = async { run(context, local_peer_id, collator_pair, collation_req_receiver, Default::default()) .await diff --git a/node/network/dispute-distribution/src/tests/mod.rs b/node/network/dispute-distribution/src/tests/mod.rs index 7abc65dbac46..18e916126289 100644 --- a/node/network/dispute-distribution/src/tests/mod.rs +++ b/node/network/dispute-distribution/src/tests/mod.rs @@ -724,7 +724,7 @@ where let keystore = make_ferdie_keystore(); let genesis_hash = Hash::repeat_byte(0xff); - let (req_receiver, req_cfg) = IncomingRequest::get_config_receiver(&genesis_hash, &None); + let (req_receiver, req_cfg) = IncomingRequest::get_config_receiver(&genesis_hash, None); let subsystem = DisputeDistributionSubsystem::new( keystore, req_receiver, diff --git a/node/network/protocol/src/request_response/incoming/mod.rs b/node/network/protocol/src/request_response/incoming/mod.rs index 38fcddb5c3b4..70f87d7e59c5 100644 --- a/node/network/protocol/src/request_response/incoming/mod.rs +++ b/node/network/protocol/src/request_response/incoming/mod.rs @@ -22,6 +22,7 @@ use futures::{ }; use parity_scale_codec::{Decode, Encode}; +use polkadot_primitives::v2::Hash; use sc_network::{config as netconfig, config::RequestResponseConfig, PeerId}; @@ -55,9 +56,9 @@ where /// /// This Register that config with substrate networking and receive incoming requests via the /// returned `IncomingRequestReceiver`. - pub fn get_config_receiver>( + pub fn get_config_receiver( genesis_hash: &Hash, - fork_id: &Option, + fork_id: Option<&str>, ) -> (IncomingRequestReceiver, RequestResponseConfig) { let (raw, cfg) = Req::PROTOCOL.get_config(genesis_hash, fork_id); (IncomingRequestReceiver { raw, phantom: PhantomData {} }, cfg) diff --git a/node/network/protocol/src/request_response/mod.rs b/node/network/protocol/src/request_response/mod.rs index 5999a8ca4587..53cef74d4be6 100644 --- a/node/network/protocol/src/request_response/mod.rs +++ b/node/network/protocol/src/request_response/mod.rs @@ -32,11 +32,11 @@ //! //! Versioned (v1 module): The actual requests and responses as sent over the network. -use std::{borrow::Cow, collections::HashMap, time::Duration, u64}; +use std::{borrow::Cow, time::Duration, u64}; use futures::channel::mpsc; -use polkadot_primitives::v2::{MAX_CODE_SIZE, MAX_POV_SIZE}; -use strum::{EnumIter, IntoEnumIterator}; +use polkadot_primitives::v2::{Hash, MAX_CODE_SIZE, MAX_POV_SIZE}; +use strum::EnumIter; pub use sc_network::{config as network, config::RequestResponseConfig}; @@ -126,13 +126,13 @@ impl Protocol { /// /// Returns a receiver for messages received on this protocol and the requested /// `ProtocolConfig`. - pub fn get_config>( + pub fn get_config( self, genesis_hash: &Hash, - fork_id: &Option, + fork_id: Option<&str>, ) -> (mpsc::Receiver, RequestResponseConfig) { - let name = self.protocol_name(genesis_hash, fork_id); - let fallback_names = self.fallback_names(); + let name = self.get_name(genesis_hash, fork_id); + let fallback_names = self.get_fallback_names(); let (tx, rx) = mpsc::channel(self.get_channel_size()); let cfg = match self { Protocol::ChunkFetchingV1 => RequestResponseConfig { @@ -249,12 +249,12 @@ impl Protocol { } /// Fallback protocol names of this protocol, as understood by substrate networking. - pub fn fallback_names(self) -> Vec> { - std::iter::once(self.legacy_name().into()).collect() + pub fn get_fallback_names(self) -> Vec> { + std::iter::once(self.get_legacy_name().into()).collect() } /// Legacy protocol name associated with each peer set. - pub const fn legacy_name(self) -> &'static str { + pub const fn get_legacy_name(self) -> &'static str { match self { Protocol::ChunkFetchingV1 => "/polkadot/req_chunk/1", Protocol::CollationFetchingV1 => "/polkadot/req_collation/1", @@ -266,11 +266,7 @@ impl Protocol { } /// Protocol name of this protocol, as understood by substrate networking. - pub fn protocol_name>( - self, - genesis_hash: &Hash, - fork_id: &Option, - ) -> Cow<'static, str> { + pub fn get_name(self, genesis_hash: &Hash, fork_id: Option<&str>) -> Cow<'static, str> { let prefix = if let Some(fork_id) = fork_id { format!("/{}/{}", hex::encode(genesis_hash), fork_id) } else { @@ -288,19 +284,6 @@ impl Protocol { format!("{}{}", prefix, short_name).into() } - - /// All protocol names as understood by substrate networking for specific `genesis_hash` - /// and `fork_id`, represented by a hash map. - pub fn protocol_names>( - genesis_hash: &Hash, - fork_id: &Option, - ) -> HashMap> { - let mut p_names = HashMap::new(); - for protocol in Protocol::iter() { - p_names.insert(protocol, protocol.protocol_name(genesis_hash, fork_id)); - } - p_names - } } /// Common properties of any `Request`. @@ -311,3 +294,18 @@ pub trait IsRequest { /// What protocol this `Request` implements. const PROTOCOL: Protocol; } + +/// Type for getting protocol names using genesis hash & fork id +pub struct ReqProtocolNames(Hash, Option); + +impl ReqProtocolNames { + /// Construct [`ReqProtocolNames`] from `genesis_hash` and `fork_id` + pub fn new(genesis_hash: Hash, fork_id: Option) -> Self { + Self(genesis_hash, fork_id) + } + + /// Get on-the-wire [`Protocol`] name + pub fn get_name(&self, protocol: Protocol) -> Cow<'static, str> { + protocol.get_name(&self.0, self.1.as_deref()) + } +} diff --git a/node/network/statement-distribution/src/tests.rs b/node/network/statement-distribution/src/tests.rs index d598421c95da..d0a9e38a458b 100644 --- a/node/network/statement-distribution/src/tests.rs +++ b/node/network/statement-distribution/src/tests.rs @@ -727,7 +727,7 @@ fn receiving_from_one_sends_to_another_and_to_candidate_backing() { let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, &None); + let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, None); let bg = async move { let s = StatementDistributionSubsystem::new( @@ -921,7 +921,7 @@ fn receiving_large_statement_from_one_sends_to_another_and_to_candidate_backing( let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); let (statement_req_receiver, mut req_cfg) = - IncomingRequest::get_config_receiver(&GENESIS_HASH, &None); + IncomingRequest::get_config_receiver(&GENESIS_HASH, None); let bg = async move { let s = StatementDistributionSubsystem::new( @@ -1434,7 +1434,7 @@ fn share_prioritizes_backing_group() { let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); let (statement_req_receiver, mut req_cfg) = - IncomingRequest::get_config_receiver(&GENESIS_HASH, &None); + IncomingRequest::get_config_receiver(&GENESIS_HASH, None); let bg = async move { let s = StatementDistributionSubsystem::new( @@ -1729,7 +1729,7 @@ fn peer_cant_flood_with_large_statements() { let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, &None); + let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, None); let bg = async move { let s = StatementDistributionSubsystem::new( make_ferdie_keystore(), @@ -1933,7 +1933,7 @@ fn handle_multiple_seconded_statements() { let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, &None); + let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, None); let virtual_overseer_fut = async move { let s = StatementDistributionSubsystem::new( diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index 0fbd64be428a..b5d898af3be6 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -859,20 +859,23 @@ where let fork_id = config.chain_spec.fork_id().map(ToOwned::to_owned); - let (pov_req_receiver, cfg) = IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); + let (pov_req_receiver, cfg) = + IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); config.network.request_response_protocols.push(cfg); - let (chunk_req_receiver, cfg) = IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); + let (chunk_req_receiver, cfg) = + IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); config.network.request_response_protocols.push(cfg); let (collation_req_receiver, cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); + IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); config.network.request_response_protocols.push(cfg); let (available_data_req_receiver, cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); + IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); config.network.request_response_protocols.push(cfg); let (statement_req_receiver, cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); + IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); config.network.request_response_protocols.push(cfg); - let (dispute_req_receiver, cfg) = IncomingRequest::get_config_receiver(&genesis_hash, &fork_id); + let (dispute_req_receiver, cfg) = + IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); config.network.request_response_protocols.push(cfg); let grandpa_hard_forks = if config.chain_spec.is_kusama() { diff --git a/node/service/src/overseer.rs b/node/service/src/overseer.rs index cecab314d542..6f07ecbd8e9f 100644 --- a/node/service/src/overseer.rs +++ b/node/service/src/overseer.rs @@ -25,7 +25,7 @@ use polkadot_node_core_candidate_validation::Config as CandidateValidationConfig use polkadot_node_core_chain_selection::Config as ChainSelectionConfig; use polkadot_node_core_dispute_coordinator::Config as DisputeCoordinatorConfig; use polkadot_node_network_protocol::request_response::{ - v1 as request_v1, IncomingRequestReceiver, Protocol as RequestsProtocol, + v1 as request_v1, IncomingRequestReceiver, ReqProtocolNames, }; #[cfg(any(feature = "malus", test))] pub use polkadot_overseer::{ @@ -200,9 +200,9 @@ where let network_bridge_metrics: NetworkBridgeMetrics = Metrics::register(registry)?; - let requests_protocols = RequestsProtocol::protocol_names( - &runtime_client.hash(0).ok().flatten().expect("Genesis block exists; qed"), - &fork_id, + let req_protocol_names = ReqProtocolNames::new( + runtime_client.hash(0).ok().flatten().expect("Genesis block exists; qed"), + fork_id, ); let builder = Overseer::builder() @@ -210,7 +210,7 @@ where network_service.clone(), authority_discovery_service.clone(), network_bridge_metrics.clone(), - requests_protocols, + req_protocol_names, )) .network_bridge_rx(NetworkBridgeRxSubsystem::new( network_service.clone(), From b208424cc738964d5c866b38601af62aade481b3 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Wed, 10 Aug 2022 21:10:46 +0300 Subject: [PATCH 07/12] Lookup protocol name in a map instead of generating it every time --- .../protocol/src/request_response/mod.rs | 21 +++++++++++++------ node/service/src/overseer.rs | 2 +- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/node/network/protocol/src/request_response/mod.rs b/node/network/protocol/src/request_response/mod.rs index 53cef74d4be6..49de49965f79 100644 --- a/node/network/protocol/src/request_response/mod.rs +++ b/node/network/protocol/src/request_response/mod.rs @@ -32,11 +32,11 @@ //! //! Versioned (v1 module): The actual requests and responses as sent over the network. -use std::{borrow::Cow, time::Duration, u64}; +use std::{borrow::Cow, collections::HashMap, time::Duration, u64}; use futures::channel::mpsc; use polkadot_primitives::v2::{Hash, MAX_CODE_SIZE, MAX_POV_SIZE}; -use strum::EnumIter; +use strum::{EnumIter, IntoEnumIterator}; pub use sc_network::{config as network, config::RequestResponseConfig}; @@ -296,16 +296,25 @@ pub trait IsRequest { } /// Type for getting protocol names using genesis hash & fork id -pub struct ReqProtocolNames(Hash, Option); +pub struct ReqProtocolNames { + names: HashMap>, +} impl ReqProtocolNames { /// Construct [`ReqProtocolNames`] from `genesis_hash` and `fork_id` - pub fn new(genesis_hash: Hash, fork_id: Option) -> Self { - Self(genesis_hash, fork_id) + pub fn new(genesis_hash: Hash, fork_id: Option<&str>) -> Self { + let mut names = HashMap::new(); + for protocol in Protocol::iter() { + names.insert(protocol, protocol.get_name(&genesis_hash, fork_id)); + } + Self { names } } /// Get on-the-wire [`Protocol`] name pub fn get_name(&self, protocol: Protocol) -> Cow<'static, str> { - protocol.get_name(&self.0, self.1.as_deref()) + self.names + .get(&protocol) + .expect("All `Protocol` enum variants are added above via `strum`; qed") + .clone() } } diff --git a/node/service/src/overseer.rs b/node/service/src/overseer.rs index 6f07ecbd8e9f..98a79403d3b5 100644 --- a/node/service/src/overseer.rs +++ b/node/service/src/overseer.rs @@ -202,7 +202,7 @@ where let req_protocol_names = ReqProtocolNames::new( runtime_client.hash(0).ok().flatten().expect("Genesis block exists; qed"), - fork_id, + fork_id.as_deref(), ); let builder = Overseer::builder() From 1ea31c3c3943a166a27a82d348eeffd4719c7e4a Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Wed, 10 Aug 2022 21:19:22 +0300 Subject: [PATCH 08/12] minor: remove unnecessary polkadot_primitives dependency --- .../protocol/src/request_response/incoming/mod.rs | 3 +-- node/network/protocol/src/request_response/mod.rs | 12 ++++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/node/network/protocol/src/request_response/incoming/mod.rs b/node/network/protocol/src/request_response/incoming/mod.rs index 70f87d7e59c5..367a05a3ee19 100644 --- a/node/network/protocol/src/request_response/incoming/mod.rs +++ b/node/network/protocol/src/request_response/incoming/mod.rs @@ -22,7 +22,6 @@ use futures::{ }; use parity_scale_codec::{Decode, Encode}; -use polkadot_primitives::v2::Hash; use sc_network::{config as netconfig, config::RequestResponseConfig, PeerId}; @@ -56,7 +55,7 @@ where /// /// This Register that config with substrate networking and receive incoming requests via the /// returned `IncomingRequestReceiver`. - pub fn get_config_receiver( + pub fn get_config_receiver>( genesis_hash: &Hash, fork_id: Option<&str>, ) -> (IncomingRequestReceiver, RequestResponseConfig) { diff --git a/node/network/protocol/src/request_response/mod.rs b/node/network/protocol/src/request_response/mod.rs index 49de49965f79..90d1bb13309b 100644 --- a/node/network/protocol/src/request_response/mod.rs +++ b/node/network/protocol/src/request_response/mod.rs @@ -35,7 +35,7 @@ use std::{borrow::Cow, collections::HashMap, time::Duration, u64}; use futures::channel::mpsc; -use polkadot_primitives::v2::{Hash, MAX_CODE_SIZE, MAX_POV_SIZE}; +use polkadot_primitives::v2::{MAX_CODE_SIZE, MAX_POV_SIZE}; use strum::{EnumIter, IntoEnumIterator}; pub use sc_network::{config as network, config::RequestResponseConfig}; @@ -126,7 +126,7 @@ impl Protocol { /// /// Returns a receiver for messages received on this protocol and the requested /// `ProtocolConfig`. - pub fn get_config( + pub fn get_config>( self, genesis_hash: &Hash, fork_id: Option<&str>, @@ -266,7 +266,11 @@ impl Protocol { } /// Protocol name of this protocol, as understood by substrate networking. - pub fn get_name(self, genesis_hash: &Hash, fork_id: Option<&str>) -> Cow<'static, str> { + pub fn get_name>( + self, + genesis_hash: &Hash, + fork_id: Option<&str>, + ) -> Cow<'static, str> { let prefix = if let Some(fork_id) = fork_id { format!("/{}/{}", hex::encode(genesis_hash), fork_id) } else { @@ -302,7 +306,7 @@ pub struct ReqProtocolNames { impl ReqProtocolNames { /// Construct [`ReqProtocolNames`] from `genesis_hash` and `fork_id` - pub fn new(genesis_hash: Hash, fork_id: Option<&str>) -> Self { + pub fn new>(genesis_hash: Hash, fork_id: Option<&str>) -> Self { let mut names = HashMap::new(); for protocol in Protocol::iter() { names.insert(protocol, protocol.get_name(&genesis_hash, fork_id)); From d7ec67be59a5009cc83dfcfa05a3107e42675b0e Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Fri, 12 Aug 2022 13:00:27 +0300 Subject: [PATCH 09/12] Make ReqProtocolNames single source of truth for req-resp protocol names --- .../src/tests/mod.rs | 7 ++- .../availability-recovery/src/tests.rs | 4 +- .../src/collator_side/tests.rs | 9 ++- .../dispute-distribution/src/tests/mod.rs | 5 +- .../src/request_response/incoming/mod.rs | 9 ++- .../protocol/src/request_response/mod.rs | 61 +++++++++---------- .../statement-distribution/src/tests.rs | 17 ++++-- node/service/src/lib.rs | 22 +++---- node/service/src/overseer.rs | 12 +--- 9 files changed, 73 insertions(+), 73 deletions(-) diff --git a/node/network/availability-distribution/src/tests/mod.rs b/node/network/availability-distribution/src/tests/mod.rs index 77b85f9a565d..ebbc436a00dd 100644 --- a/node/network/availability-distribution/src/tests/mod.rs +++ b/node/network/availability-distribution/src/tests/mod.rs @@ -18,7 +18,7 @@ use std::collections::HashSet; use futures::{executor, future, Future}; -use polkadot_node_network_protocol::request_response::IncomingRequest; +use polkadot_node_network_protocol::request_response::{IncomingRequest, ReqProtocolNames}; use polkadot_primitives::v2::{CoreState, Hash}; use sp_keystore::SyncCryptoStorePtr; @@ -42,10 +42,11 @@ fn test_harness>( let pool = sp_core::testing::TaskExecutor::new(); let (context, virtual_overseer) = test_helpers::make_subsystem_context(pool.clone()); let genesis_hash = Hash::repeat_byte(0xff); + let req_protocol_names = ReqProtocolNames::new(&genesis_hash, None); - let (pov_req_receiver, pov_req_cfg) = IncomingRequest::get_config_receiver(&genesis_hash, None); + let (pov_req_receiver, pov_req_cfg) = IncomingRequest::get_config_receiver(&req_protocol_names); let (chunk_req_receiver, chunk_req_cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, None); + IncomingRequest::get_config_receiver(&req_protocol_names); let subsystem = AvailabilityDistributionSubsystem::new( keystore, IncomingRequestReceivers { pov_req_receiver, chunk_req_receiver }, diff --git a/node/network/availability-recovery/src/tests.rs b/node/network/availability-recovery/src/tests.rs index ae8b12ae6e9d..b412887ebc91 100644 --- a/node/network/availability-recovery/src/tests.rs +++ b/node/network/availability-recovery/src/tests.rs @@ -21,7 +21,7 @@ use futures::{executor, future}; use futures_timer::Delay; use parity_scale_codec::Encode; -use polkadot_node_network_protocol::request_response::IncomingRequest; +use polkadot_node_network_protocol::request_response::{IncomingRequest, ReqProtocolNames}; use super::*; @@ -92,7 +92,7 @@ fn test_harness_chunks_only>( let (context, virtual_overseer) = test_helpers::make_subsystem_context(pool.clone()); let genesis_hash = Hash::repeat_byte(0xff); + let req_protocol_names = ReqProtocolNames::new(&genesis_hash, None); let (collation_req_receiver, req_cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, None); + IncomingRequest::get_config_receiver(&req_protocol_names); let subsystem = async { run(context, local_peer_id, collator_pair, collation_req_receiver, Default::default()) .await diff --git a/node/network/dispute-distribution/src/tests/mod.rs b/node/network/dispute-distribution/src/tests/mod.rs index 18e916126289..8f78b664de82 100644 --- a/node/network/dispute-distribution/src/tests/mod.rs +++ b/node/network/dispute-distribution/src/tests/mod.rs @@ -31,7 +31,7 @@ use parity_scale_codec::{Decode, Encode}; use sc_network::config::RequestResponseConfig; use polkadot_node_network_protocol::{ - request_response::{v1::DisputeRequest, IncomingRequest}, + request_response::{v1::DisputeRequest, IncomingRequest, ReqProtocolNames}, PeerId, }; use sp_keyring::Sr25519Keyring; @@ -724,7 +724,8 @@ where let keystore = make_ferdie_keystore(); let genesis_hash = Hash::repeat_byte(0xff); - let (req_receiver, req_cfg) = IncomingRequest::get_config_receiver(&genesis_hash, None); + let req_protocol_names = ReqProtocolNames::new(&genesis_hash, None); + let (req_receiver, req_cfg) = IncomingRequest::get_config_receiver(&req_protocol_names); let subsystem = DisputeDistributionSubsystem::new( keystore, req_receiver, diff --git a/node/network/protocol/src/request_response/incoming/mod.rs b/node/network/protocol/src/request_response/incoming/mod.rs index 367a05a3ee19..808d70645995 100644 --- a/node/network/protocol/src/request_response/incoming/mod.rs +++ b/node/network/protocol/src/request_response/incoming/mod.rs @@ -25,7 +25,7 @@ use parity_scale_codec::{Decode, Encode}; use sc_network::{config as netconfig, config::RequestResponseConfig, PeerId}; -use super::IsRequest; +use super::{IsRequest, ReqProtocolNames}; use crate::UnifiedReputationChange; mod error; @@ -55,11 +55,10 @@ where /// /// This Register that config with substrate networking and receive incoming requests via the /// returned `IncomingRequestReceiver`. - pub fn get_config_receiver>( - genesis_hash: &Hash, - fork_id: Option<&str>, + pub fn get_config_receiver( + req_protocol_names: &ReqProtocolNames, ) -> (IncomingRequestReceiver, RequestResponseConfig) { - let (raw, cfg) = Req::PROTOCOL.get_config(genesis_hash, fork_id); + let (raw, cfg) = Req::PROTOCOL.get_config(req_protocol_names); (IncomingRequestReceiver { raw, phantom: PhantomData {} }, cfg) } diff --git a/node/network/protocol/src/request_response/mod.rs b/node/network/protocol/src/request_response/mod.rs index 90d1bb13309b..64cce156a9d5 100644 --- a/node/network/protocol/src/request_response/mod.rs +++ b/node/network/protocol/src/request_response/mod.rs @@ -126,12 +126,11 @@ impl Protocol { /// /// Returns a receiver for messages received on this protocol and the requested /// `ProtocolConfig`. - pub fn get_config>( + pub fn get_config( self, - genesis_hash: &Hash, - fork_id: Option<&str>, + req_protocol_names: &ReqProtocolNames, ) -> (mpsc::Receiver, RequestResponseConfig) { - let name = self.get_name(genesis_hash, fork_id); + let name = req_protocol_names.get_name(self); let fallback_names = self.get_fallback_names(); let (tx, rx) = mpsc::channel(self.get_channel_size()); let cfg = match self { @@ -249,12 +248,12 @@ impl Protocol { } /// Fallback protocol names of this protocol, as understood by substrate networking. - pub fn get_fallback_names(self) -> Vec> { + fn get_fallback_names(self) -> Vec> { std::iter::once(self.get_legacy_name().into()).collect() } /// Legacy protocol name associated with each peer set. - pub const fn get_legacy_name(self) -> &'static str { + const fn get_legacy_name(self) -> &'static str { match self { Protocol::ChunkFetchingV1 => "/polkadot/req_chunk/1", Protocol::CollationFetchingV1 => "/polkadot/req_collation/1", @@ -264,30 +263,6 @@ impl Protocol { Protocol::DisputeSendingV1 => "/polkadot/send_dispute/1", } } - - /// Protocol name of this protocol, as understood by substrate networking. - pub fn get_name>( - self, - genesis_hash: &Hash, - fork_id: Option<&str>, - ) -> Cow<'static, str> { - let prefix = if let Some(fork_id) = fork_id { - format!("/{}/{}", hex::encode(genesis_hash), fork_id) - } else { - format!("/{}", hex::encode(genesis_hash)) - }; - - let short_name = match self { - Protocol::ChunkFetchingV1 => "/req_chunk/1", - Protocol::CollationFetchingV1 => "/req_collation/1", - Protocol::PoVFetchingV1 => "/req_pov/1", - Protocol::AvailableDataFetchingV1 => "/req_available_data/1", - Protocol::StatementFetchingV1 => "/req_statement/1", - Protocol::DisputeSendingV1 => "/send_dispute/1", - }; - - format!("{}{}", prefix, short_name).into() - } } /// Common properties of any `Request`. @@ -309,7 +284,7 @@ impl ReqProtocolNames { pub fn new>(genesis_hash: Hash, fork_id: Option<&str>) -> Self { let mut names = HashMap::new(); for protocol in Protocol::iter() { - names.insert(protocol, protocol.get_name(&genesis_hash, fork_id)); + names.insert(protocol, Self::generate_name(protocol, &genesis_hash, fork_id)); } Self { names } } @@ -321,4 +296,28 @@ impl ReqProtocolNames { .expect("All `Protocol` enum variants are added above via `strum`; qed") .clone() } + + /// Protocol name of this protocol based on `genesis_hash` and `fork_id`. + fn generate_name>( + protocol: Protocol, + genesis_hash: &Hash, + fork_id: Option<&str>, + ) -> Cow<'static, str> { + let prefix = if let Some(fork_id) = fork_id { + format!("/{}/{}", hex::encode(genesis_hash), fork_id) + } else { + format!("/{}", hex::encode(genesis_hash)) + }; + + let short_name = match protocol { + Protocol::ChunkFetchingV1 => "/req_chunk/1", + Protocol::CollationFetchingV1 => "/req_collation/1", + Protocol::PoVFetchingV1 => "/req_pov/1", + Protocol::AvailableDataFetchingV1 => "/req_available_data/1", + Protocol::StatementFetchingV1 => "/req_statement/1", + Protocol::DisputeSendingV1 => "/send_dispute/1", + }; + + format!("{}{}", prefix, short_name).into() + } } diff --git a/node/network/statement-distribution/src/tests.rs b/node/network/statement-distribution/src/tests.rs index d0a9e38a458b..efc9ca896bb1 100644 --- a/node/network/statement-distribution/src/tests.rs +++ b/node/network/statement-distribution/src/tests.rs @@ -22,7 +22,7 @@ use parity_scale_codec::{Decode, Encode}; use polkadot_node_network_protocol::{ request_response::{ v1::{StatementFetchingRequest, StatementFetchingResponse}, - IncomingRequest, Recipient, Requests, + IncomingRequest, Recipient, ReqProtocolNames, Requests, }, view, ObservedRole, }; @@ -727,7 +727,8 @@ fn receiving_from_one_sends_to_another_and_to_candidate_backing() { let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, None); + let req_protocol_names = ReqProtocolNames::new(&GENESIS_HASH, None); + let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&req_protocol_names); let bg = async move { let s = StatementDistributionSubsystem::new( @@ -920,8 +921,9 @@ fn receiving_large_statement_from_one_sends_to_another_and_to_candidate_backing( let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); + let req_protocol_names = ReqProtocolNames::new(&GENESIS_HASH, None); let (statement_req_receiver, mut req_cfg) = - IncomingRequest::get_config_receiver(&GENESIS_HASH, None); + IncomingRequest::get_config_receiver(&req_protocol_names); let bg = async move { let s = StatementDistributionSubsystem::new( @@ -1433,8 +1435,9 @@ fn share_prioritizes_backing_group() { let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); + let req_protocol_names = ReqProtocolNames::new(&GENESIS_HASH, None); let (statement_req_receiver, mut req_cfg) = - IncomingRequest::get_config_receiver(&GENESIS_HASH, None); + IncomingRequest::get_config_receiver(&req_protocol_names); let bg = async move { let s = StatementDistributionSubsystem::new( @@ -1729,7 +1732,8 @@ fn peer_cant_flood_with_large_statements() { let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, None); + let req_protocol_names = ReqProtocolNames::new(&GENESIS_HASH, None); + let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&req_protocol_names); let bg = async move { let s = StatementDistributionSubsystem::new( make_ferdie_keystore(), @@ -1933,7 +1937,8 @@ fn handle_multiple_seconded_statements() { let pool = sp_core::testing::TaskExecutor::new(); let (ctx, mut handle) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool); - let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&GENESIS_HASH, None); + let req_protocol_names = ReqProtocolNames::new(&GENESIS_HASH, None); + let (statement_req_receiver, _) = IncomingRequest::get_config_receiver(&req_protocol_names); let virtual_overseer_fut = async move { let s = StatementDistributionSubsystem::new( diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index b5d898af3be6..70e0aa34e194 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -46,6 +46,7 @@ use { self as chain_selection_subsystem, Config as ChainSelectionConfig, }, polkadot_node_core_dispute_coordinator::Config as DisputeCoordinatorConfig, + polkadot_node_network_protocol::request_response::ReqProtocolNames, polkadot_overseer::BlockInfo, sc_client_api::{BlockBackend, ExecutorProvider}, sp_core::traits::SpawnNamed, @@ -857,25 +858,20 @@ where config.network.extra_sets.extend(peer_sets_info(is_authority)); } - let fork_id = config.chain_spec.fork_id().map(ToOwned::to_owned); + let req_protocol_names = ReqProtocolNames::new(&genesis_hash, config.chain_spec.fork_id()); - let (pov_req_receiver, cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); + let (pov_req_receiver, cfg) = IncomingRequest::get_config_receiver(&req_protocol_names); config.network.request_response_protocols.push(cfg); - let (chunk_req_receiver, cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); + let (chunk_req_receiver, cfg) = IncomingRequest::get_config_receiver(&req_protocol_names); config.network.request_response_protocols.push(cfg); - let (collation_req_receiver, cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); + let (collation_req_receiver, cfg) = IncomingRequest::get_config_receiver(&req_protocol_names); config.network.request_response_protocols.push(cfg); let (available_data_req_receiver, cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); + IncomingRequest::get_config_receiver(&req_protocol_names); config.network.request_response_protocols.push(cfg); - let (statement_req_receiver, cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); + let (statement_req_receiver, cfg) = IncomingRequest::get_config_receiver(&req_protocol_names); config.network.request_response_protocols.push(cfg); - let (dispute_req_receiver, cfg) = - IncomingRequest::get_config_receiver(&genesis_hash, fork_id.as_deref()); + let (dispute_req_receiver, cfg) = IncomingRequest::get_config_receiver(&req_protocol_names); config.network.request_response_protocols.push(cfg); let grandpa_hard_forks = if config.chain_spec.is_kusama() { @@ -1065,7 +1061,7 @@ where dispute_coordinator_config, pvf_checker_enabled, overseer_message_channel_capacity_override, - fork_id, + req_protocol_names, }, ) .map_err(|e| { diff --git a/node/service/src/overseer.rs b/node/service/src/overseer.rs index 98a79403d3b5..e502c0ac698c 100644 --- a/node/service/src/overseer.rs +++ b/node/service/src/overseer.rs @@ -119,9 +119,8 @@ where pub pvf_checker_enabled: bool, /// Overseer channel capacity override. pub overseer_message_channel_capacity_override: Option, - /// Fork ID from the client spec to only talk to "our" nodes if multiple chains - /// have the same genesis hash. - pub fork_id: Option, + /// Request-response protocol names source. + pub req_protocol_names: ReqProtocolNames, } /// Obtain a prepared `OverseerBuilder`, that is initialized @@ -150,7 +149,7 @@ pub fn prepared_overseer_builder<'a, Spawner, RuntimeClient>( dispute_coordinator_config, pvf_checker_enabled, overseer_message_channel_capacity_override, - fork_id, + req_protocol_names, }: OverseerGenArgs<'a, Spawner, RuntimeClient>, ) -> Result< InitializedOverseerBuilder< @@ -200,11 +199,6 @@ where let network_bridge_metrics: NetworkBridgeMetrics = Metrics::register(registry)?; - let req_protocol_names = ReqProtocolNames::new( - runtime_client.hash(0).ok().flatten().expect("Genesis block exists; qed"), - fork_id.as_deref(), - ); - let builder = Overseer::builder() .network_bridge_tx(NetworkBridgeTxSubsystem::new( network_service.clone(), From 6be182ec922f0717e4fbf881e9d558103d2f9022 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Fri, 12 Aug 2022 13:07:05 +0300 Subject: [PATCH 10/12] minor: spelling --- node/network/protocol/src/request_response/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/node/network/protocol/src/request_response/mod.rs b/node/network/protocol/src/request_response/mod.rs index 64cce156a9d5..0b308c0012a8 100644 --- a/node/network/protocol/src/request_response/mod.rs +++ b/node/network/protocol/src/request_response/mod.rs @@ -274,13 +274,13 @@ pub trait IsRequest { const PROTOCOL: Protocol; } -/// Type for getting protocol names using genesis hash & fork id +/// Type for getting protocol names using genesis hash & fork id. pub struct ReqProtocolNames { names: HashMap>, } impl ReqProtocolNames { - /// Construct [`ReqProtocolNames`] from `genesis_hash` and `fork_id` + /// Construct [`ReqProtocolNames`] from `genesis_hash` and `fork_id`. pub fn new>(genesis_hash: Hash, fork_id: Option<&str>) -> Self { let mut names = HashMap::new(); for protocol in Protocol::iter() { @@ -289,7 +289,7 @@ impl ReqProtocolNames { Self { names } } - /// Get on-the-wire [`Protocol`] name + /// Get on-the-wire [`Protocol`] name. pub fn get_name(&self, protocol: Protocol) -> Cow<'static, str> { self.names .get(&protocol) From 4f23a50a117c53a9ec215d3bafc69d5762e581b0 Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Fri, 12 Aug 2022 13:09:50 +0300 Subject: [PATCH 11/12] minor: docs --- node/network/protocol/src/request_response/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node/network/protocol/src/request_response/mod.rs b/node/network/protocol/src/request_response/mod.rs index 0b308c0012a8..fb955286990e 100644 --- a/node/network/protocol/src/request_response/mod.rs +++ b/node/network/protocol/src/request_response/mod.rs @@ -274,7 +274,7 @@ pub trait IsRequest { const PROTOCOL: Protocol; } -/// Type for getting protocol names using genesis hash & fork id. +/// Type for getting on the wire [`Protocol`] names using genesis hash & fork id. pub struct ReqProtocolNames { names: HashMap>, } @@ -289,7 +289,7 @@ impl ReqProtocolNames { Self { names } } - /// Get on-the-wire [`Protocol`] name. + /// Get on the wire [`Protocol`] name. pub fn get_name(&self, protocol: Protocol) -> Cow<'static, str> { self.names .get(&protocol) From 44a7e59ee6a8b44de0826c4f7ed1461284d53afb Mon Sep 17 00:00:00 2001 From: Dmitry Markin Date: Fri, 12 Aug 2022 14:21:20 +0300 Subject: [PATCH 12/12] Fix test --- node/network/availability-recovery/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/network/availability-recovery/src/tests.rs b/node/network/availability-recovery/src/tests.rs index b412887ebc91..2cfed743bc5e 100644 --- a/node/network/availability-recovery/src/tests.rs +++ b/node/network/availability-recovery/src/tests.rs @@ -57,7 +57,7 @@ fn test_harness_fast_path