Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Change request-response protocol names to include genesis hash & fork id #5870

Merged
merged 12 commits into from
Aug 12, 2022
Merged
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 6 additions & 3 deletions node/network/availability-distribution/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,9 +41,12 @@ fn test_harness<T: Future<Output = ()>>(

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 },
Expand Down
11 changes: 8 additions & 3 deletions node/network/availability-recovery/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<AvailabilityRecoveryMessage>;

// Deterministic genesis hash for protocol names
const GENESIS_HASH: Hash = Hash::repeat_byte(0xff);

fn test_harness_fast_path<T: Future<Output = (VirtualOverseer, RequestResponseConfig)>>(
test: impl FnOnce(VirtualOverseer, RequestResponseConfig) -> T,
) {
Expand All @@ -53,7 +56,8 @@ fn test_harness_fast_path<T: Future<Output = (VirtualOverseer, RequestResponseCo

let (context, virtual_overseer) = make_subsystem_context(pool.clone());

let (collation_req_receiver, req_cfg) = IncomingRequest::get_config_receiver();
let (collation_req_receiver, req_cfg) =
IncomingRequest::get_config_receiver(&GENESIS_HASH, &None);
let subsystem =
AvailabilityRecoverySubsystem::with_fast_path(collation_req_receiver, Metrics::new_dummy());
let subsystem = async {
Expand Down Expand Up @@ -87,7 +91,8 @@ fn test_harness_chunks_only<T: Future<Output = (VirtualOverseer, RequestResponse

let (context, virtual_overseer) = make_subsystem_context(pool.clone());

let (collation_req_receiver, req_cfg) = IncomingRequest::get_config_receiver();
let (collation_req_receiver, req_cfg) =
IncomingRequest::get_config_receiver(&GENESIS_HASH, &None);
let subsystem = AvailabilityRecoverySubsystem::with_chunks_only(
collation_req_receiver,
Metrics::new_dummy(),
Expand Down
15 changes: 12 additions & 3 deletions node/network/bridge/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

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};
Expand All @@ -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};
Expand Down Expand Up @@ -97,6 +101,7 @@ pub trait Network: Clone + Send + 'static {
&self,
authority_discovery: &mut AD,
req: Requests,
req_protocols: &HashMap<Protocol, Cow<'static, str>>,
if_disconnected: IfDisconnected,
);

Expand Down Expand Up @@ -153,6 +158,7 @@ impl Network for Arc<NetworkService<Block, Hash>> {
&self,
authority_discovery: &mut AD,
req: Requests,
req_protocols: &HashMap<Protocol, Cow<'static, str>>,
if_disconnected: IfDisconnected,
) {
let (protocol, OutgoingRequest { peer, payload, pending_response }) = req.encode_request();
Expand Down Expand Up @@ -198,7 +204,10 @@ impl Network for Arc<NetworkService<Block, Hash>> {
NetworkService::start_request(
&*self,
peer_id,
protocol.into_protocol_name(),
req_protocols
dmitry-markin marked this conversation as resolved.
Show resolved Hide resolved
.get(&protocol)
.expect("all `protocol` names are generated via `strum`; qed")
.clone(),
payload,
pending_response,
if_disconnected,
Expand Down
3 changes: 2 additions & 1 deletion node/network/bridge/src/rx/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

use super::*;
use futures::{channel::oneshot, executor, stream::BoxStream};
use polkadot_node_network_protocol::{self as net_protocol, OurView};
use polkadot_node_network_protocol::{self as net_protocol, request_response::Protocol, OurView};
use polkadot_node_subsystem::{messages::NetworkBridgeEvent, ActivatedLeaf};

use assert_matches::assert_matches;
Expand Down Expand Up @@ -117,6 +117,7 @@ impl Network for TestNetwork {
&self,
_: &mut AD,
_: Requests,
_: &HashMap<Protocol, Cow<'static, str>>,
_: IfDisconnected,
) {
}
Expand Down
44 changes: 37 additions & 7 deletions node/network/bridge/src/tx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,13 @@
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! 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,
Expand Down Expand Up @@ -50,15 +54,21 @@ pub struct NetworkBridgeTx<N, AD> {
network_service: N,
authority_discovery_service: AD,
metrics: Metrics,
requests_protocols: HashMap<Protocol, Cow<'static, str>>,
}

impl<N, AD> NetworkBridgeTx<N, AD> {
/// Create a new network bridge subsystem with underlying network service and authority discovery service.
///
/// 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<Protocol, Cow<'static, str>>,
) -> Self {
Self { network_service, authority_discovery_service, metrics, requests_protocols }
}
}

Expand All @@ -82,6 +92,7 @@ async fn handle_subsystem_messages<Context, N, AD>(
mut network_service: N,
mut authority_discovery_service: AD,
metrics: Metrics,
requests_protocols: &HashMap<Protocol, Cow<'static, str>>,
) -> Result<(), Error>
where
N: Network,
Expand All @@ -102,6 +113,7 @@ where
authority_discovery_service.clone(),
msg,
&metrics,
requests_protocols,
)
.await;
},
Expand All @@ -117,6 +129,7 @@ async fn handle_incoming_subsystem_communication<Context, N, AD>(
mut authority_discovery_service: AD,
msg: NetworkBridgeTxMessage,
metrics: &Metrics,
requests_protocols: &HashMap<Protocol, Cow<'static, str>>,
) -> (N, AD)
where
N: Network,
Expand Down Expand Up @@ -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;
}
},
Expand Down Expand Up @@ -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(())
}
Expand Down
8 changes: 6 additions & 2 deletions node/network/bridge/src/tx/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -104,6 +104,7 @@ impl Network for TestNetwork {
&self,
_: &mut AD,
_: Requests,
_: &HashMap<Protocol, Cow<'static, str>>,
_: IfDisconnected,
) {
}
Expand Down Expand Up @@ -182,7 +183,10 @@ fn test_harness<T: Future<Output = VirtualOverseer>>(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))
Expand Down
6 changes: 5 additions & 1 deletion node/network/bridge/src/validator_discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -236,6 +239,7 @@ mod tests {
&self,
_: &mut AD,
_: Requests,
_: &HashMap<Protocol, Cow<'static, str>>,
_: IfDisconnected,
) {
}
Expand Down
5 changes: 4 additions & 1 deletion node/network/collator-protocol/src/collator_side/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,10 @@ fn test_harness<T: Future<Output = TestHarness>>(

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
Expand Down
3 changes: 2 additions & 1 deletion node/network/dispute-distribution/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions node/network/protocol/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
7 changes: 5 additions & 2 deletions node/network/protocol/src/request_response/incoming/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Req>, RequestResponseConfig) {
let (raw, cfg) = Req::PROTOCOL.get_config();
pub fn get_config_receiver<Hash: AsRef<[u8]>>(
genesis_hash: &Hash,
fork_id: &Option<String>,
) -> (IncomingRequestReceiver<Req>, RequestResponseConfig) {
let (raw, cfg) = Req::PROTOCOL.get_config(genesis_hash, fork_id);
(IncomingRequestReceiver { raw, phantom: PhantomData {} }, cfg)
}

Expand Down