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

client/*: Treat protocol name as str and not [u8] #6967

Merged
3 commits merged into from
Aug 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/finality-grandpa/src/communication/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ mod periodic;
pub(crate) mod tests;

pub use sp_finality_grandpa::GRANDPA_ENGINE_ID;
pub const GRANDPA_PROTOCOL_NAME: &[u8] = b"/paritytech/grandpa/1";
pub const GRANDPA_PROTOCOL_NAME: &'static str = "/paritytech/grandpa/1";

// cost scalars for reporting peers.
mod cost {
Expand Down
2 changes: 1 addition & 1 deletion client/finality-grandpa/src/communication/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl sc_network_gossip::Network<Block> for TestNetwork {
let _ = self.sender.unbounded_send(Event::WriteNotification(who, message));
}

fn register_notifications_protocol(&self, _: ConsensusEngineId, _: Cow<'static, [u8]>) {}
fn register_notifications_protocol(&self, _: ConsensusEngineId, _: Cow<'static, str>) {}

fn announce(&self, block: Hash, _associated_data: Vec<u8>) {
let _ = self.sender.unbounded_send(Event::Announce(block));
Expand Down
10 changes: 5 additions & 5 deletions client/network-gossip/src/bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl<B: BlockT> GossipEngine<B> {
pub fn new<N: Network<B> + Send + Clone + 'static>(
network: N,
engine_id: ConsensusEngineId,
protocol_name: impl Into<Cow<'static, [u8]>>,
protocol_name: impl Into<Cow<'static, str>>,
validator: Arc<dyn Validator<B>>,
) -> Self where B: 'static {
// We grab the event stream before registering the notifications protocol, otherwise we
Expand Down Expand Up @@ -333,7 +333,7 @@ mod tests {
unimplemented!();
}

fn register_notifications_protocol(&self, _: ConsensusEngineId, _: Cow<'static, [u8]>) {}
fn register_notifications_protocol(&self, _: ConsensusEngineId, _: Cow<'static, str>) {}

fn announce(&self, _: B::Hash, _: Vec<u8>) {
unimplemented!();
Expand Down Expand Up @@ -362,7 +362,7 @@ mod tests {
let mut gossip_engine = GossipEngine::<Block>::new(
network.clone(),
[1, 2, 3, 4],
"my_protocol".as_bytes(),
"my_protocol",
Arc::new(AllowAll{}),
);

Expand Down Expand Up @@ -390,7 +390,7 @@ mod tests {
let mut gossip_engine = GossipEngine::<Block>::new(
network.clone(),
engine_id.clone(),
"my_protocol".as_bytes(),
"my_protocol",
Arc::new(AllowAll{}),
);

Expand Down Expand Up @@ -525,7 +525,7 @@ mod tests {
let mut gossip_engine = GossipEngine::<Block>::new(
network.clone(),
engine_id.clone(),
"my_protocol".as_bytes(),
"my_protocol",
Arc::new(TestValidator{}),
);

Expand Down
4 changes: 2 additions & 2 deletions client/network-gossip/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub trait Network<B: BlockT> {
fn register_notifications_protocol(
&self,
engine_id: ConsensusEngineId,
protocol_name: Cow<'static, [u8]>,
protocol_name: Cow<'static, str>,
);

/// Notify everyone we're connected to that we have the given block.
Expand Down Expand Up @@ -117,7 +117,7 @@ impl<B: BlockT, H: ExHashT> Network<B> for Arc<NetworkService<B, H>> {
fn register_notifications_protocol(
&self,
engine_id: ConsensusEngineId,
protocol_name: Cow<'static, [u8]>,
protocol_name: Cow<'static, str>,
) {
NetworkService::register_notifications_protocol(self, engine_id, protocol_name)
}
Expand Down
2 changes: 1 addition & 1 deletion client/network-gossip/src/state_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ mod tests {
unimplemented!();
}

fn register_notifications_protocol(&self, _: ConsensusEngineId, _: Cow<'static, [u8]>) {}
fn register_notifications_protocol(&self, _: ConsensusEngineId, _: Cow<'static, str>) {}

fn announce(&self, _: B::Hash, _: Vec<u8>) {
unimplemented!();
Expand Down
2 changes: 1 addition & 1 deletion client/network/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ impl<B: BlockT, H: ExHashT> Behaviour<B, H> {
pub fn register_notifications_protocol(
&mut self,
engine_id: ConsensusEngineId,
protocol_name: impl Into<Cow<'static, [u8]>>,
protocol_name: impl Into<Cow<'static, str>>,
) {
// This is the message that we will send to the remote as part of the initial handshake.
// At the moment, we force this to be an encoded `Roles`.
Expand Down
2 changes: 1 addition & 1 deletion client/network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ pub struct NetworkConfiguration {
pub node_key: NodeKeyConfig,
/// List of notifications protocols that the node supports. Must also include a
/// `ConsensusEngineId` for backwards-compatibility.
pub notifications_protocols: Vec<(ConsensusEngineId, Cow<'static, [u8]>)>,
pub notifications_protocols: Vec<(ConsensusEngineId, Cow<'static, str>)>,
/// List of request-response protocols that the node supports.
pub request_response_protocols: Vec<RequestResponseConfig>,
/// Maximum allowed number of incoming connections.
Expand Down
4 changes: 2 additions & 2 deletions client/network/src/gossip/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,14 @@ fn build_nodes_one_proto()
let listen_addr = config::build_multiaddr![Memory(rand::random::<u64>())];

let (node1, events_stream1) = build_test_full_node(config::NetworkConfiguration {
notifications_protocols: vec![(ENGINE_ID, From::from(&b"/foo"[..]))],
notifications_protocols: vec![(ENGINE_ID, From::from("/foo"))],
listen_addresses: vec![listen_addr.clone()],
transport: config::TransportConfig::MemoryOnly,
.. config::NetworkConfiguration::new_local()
});

let (node2, events_stream2) = build_test_full_node(config::NetworkConfiguration {
notifications_protocols: vec![(ENGINE_ID, From::from(&b"/foo"[..]))],
notifications_protocols: vec![(ENGINE_ID, From::from("/foo"))],
listen_addresses: vec![],
reserved_nodes: vec![config::MultiaddrWithPeerId {
multiaddr: listen_addr,
Expand Down
32 changes: 17 additions & 15 deletions client/network/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,13 +245,13 @@ pub struct Protocol<B: BlockT, H: ExHashT> {
/// Handles opening the unique substream and sending and receiving raw messages.
behaviour: GenericProto,
/// For each legacy gossiping engine ID, the corresponding new protocol name.
protocol_name_by_engine: HashMap<ConsensusEngineId, Cow<'static, [u8]>>,
protocol_name_by_engine: HashMap<ConsensusEngineId, Cow<'static, str>>,
/// For each protocol name, the legacy equivalent.
legacy_equiv_by_name: HashMap<Cow<'static, [u8]>, Fallback>,
legacy_equiv_by_name: HashMap<Cow<'static, str>, Fallback>,
/// Name of the protocol used for transactions.
transactions_protocol: Cow<'static, [u8]>,
transactions_protocol: Cow<'static, str>,
/// Name of the protocol used for block announces.
block_announces_protocol: Cow<'static, [u8]>,
block_announces_protocol: Cow<'static, str>,
/// Prometheus metrics.
metrics: Option<Metrics>,
/// The `PeerId`'s of all boot nodes.
Expand Down Expand Up @@ -417,19 +417,21 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {

let mut legacy_equiv_by_name = HashMap::new();

let transactions_protocol: Cow<'static, [u8]> = Cow::from({
let mut proto = b"/".to_vec();
proto.extend(protocol_id.as_ref().as_bytes());
proto.extend(b"/transactions/1");
let transactions_protocol: Cow<'static, str> = Cow::from({
let mut proto = String::new();
proto.push_str("/");
proto.push_str(protocol_id.as_ref());
proto.push_str("/transactions/1");
proto
});
behaviour.register_notif_protocol(transactions_protocol.clone(), Vec::new());
legacy_equiv_by_name.insert(transactions_protocol.clone(), Fallback::Transactions);

let block_announces_protocol: Cow<'static, [u8]> = Cow::from({
let mut proto = b"/".to_vec();
proto.extend(protocol_id.as_ref().as_bytes());
proto.extend(b"/block-announces/1");
let block_announces_protocol: Cow<'static, str> = Cow::from({
let mut proto = String::new();
proto.push_str("/");
proto.push_str(protocol_id.as_ref());
proto.push_str("/block-announces/1");
proto
});
behaviour.register_notif_protocol(
Expand Down Expand Up @@ -679,7 +681,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
fn send_message(
&mut self,
who: &PeerId,
message: Option<(Cow<'static, [u8]>, Vec<u8>)>,
message: Option<(Cow<'static, str>, Vec<u8>)>,
legacy: Message<B>,
) {
send_message::<B>(
Expand Down Expand Up @@ -1076,7 +1078,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
pub fn register_notifications_protocol<'a>(
&'a mut self,
engine_id: ConsensusEngineId,
protocol_name: impl Into<Cow<'static, [u8]>>,
protocol_name: impl Into<Cow<'static, str>>,
handshake_message: Vec<u8>,
) -> impl Iterator<Item = (&'a PeerId, Roles, &'a NotificationsSink)> + 'a {
let protocol_name = protocol_name.into();
Expand Down Expand Up @@ -1607,7 +1609,7 @@ fn send_message<B: BlockT>(
behaviour: &mut GenericProto,
stats: &mut HashMap<&'static str, PacketStats>,
who: &PeerId,
message: Option<(Cow<'static, [u8]>, Vec<u8>)>,
message: Option<(Cow<'static, str>, Vec<u8>)>,
legacy_message: Message<B>,
) {
let encoded = legacy_message.encode();
Expand Down
18 changes: 9 additions & 9 deletions client/network/src/protocol/generic_proto/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pub struct GenericProto {
/// Notification protocols. Entries are only ever added and not removed.
/// Contains, for each protocol, the protocol name and the message to send as part of the
/// initial handshake.
notif_protocols: Vec<(Cow<'static, [u8]>, Arc<RwLock<Vec<u8>>>)>,
notif_protocols: Vec<(Cow<'static, str>, Arc<RwLock<Vec<u8>>>)>,

/// Receiver for instructions about who to connect to or disconnect from.
peerset: sc_peerset::Peerset,
Expand Down Expand Up @@ -322,7 +322,7 @@ pub enum GenericProtoOut {
/// Id of the peer the message came from.
peer_id: PeerId,
/// Engine corresponding to the message.
protocol_name: Cow<'static, [u8]>,
protocol_name: Cow<'static, str>,
/// Message that has been received.
message: BytesMut,
},
Expand Down Expand Up @@ -360,7 +360,7 @@ impl GenericProto {
/// will retain the protocols that were registered then, and not any new one.
pub fn register_notif_protocol(
&mut self,
protocol_name: impl Into<Cow<'static, [u8]>>,
protocol_name: impl Into<Cow<'static, str>>,
handshake_msg: impl Into<Vec<u8>>
) {
self.notif_protocols.push((protocol_name.into(), Arc::new(RwLock::new(handshake_msg.into()))));
Expand All @@ -371,10 +371,10 @@ impl GenericProto {
/// Has no effect if the protocol is unknown.
pub fn set_notif_protocol_handshake(
&mut self,
protocol_name: &[u8],
protocol_name: &str,
handshake_message: impl Into<Vec<u8>>
) {
if let Some(protocol) = self.notif_protocols.iter_mut().find(|(name, _)| name == &protocol_name) {
if let Some(protocol) = self.notif_protocols.iter_mut().find(|(name, _)| name == protocol_name) {
*protocol.1.write() = handshake_message.into();
}
}
Expand Down Expand Up @@ -551,7 +551,7 @@ impl GenericProto {
pub fn write_notification(
&mut self,
target: &PeerId,
protocol_name: Cow<'static, [u8]>,
protocol_name: Cow<'static, str>,
message: impl Into<Vec<u8>>,
encoded_fallback_message: Vec<u8>,
) {
Expand All @@ -569,11 +569,11 @@ impl GenericProto {
target: "sub-libp2p",
"External API => Notification({:?}, {:?})",
target,
str::from_utf8(&protocol_name)
protocol_name,
);
trace!(target: "sub-libp2p", "Handler({:?}) <= Packet", target);
notifs_sink.send_sync_notification(
&protocol_name,
protocol_name,
encoded_fallback_message,
message
);
Expand Down Expand Up @@ -1374,7 +1374,7 @@ impl NetworkBehaviour for GenericProto {
target: "sub-libp2p",
"Handler({:?}) => Notification({:?})",
source,
str::from_utf8(&protocol_name)
protocol_name,
);
trace!(target: "sub-libp2p", "External API <= Message({:?}, {:?})", protocol_name, source);
let event = GenericProtoOut::Notification {
Expand Down
20 changes: 10 additions & 10 deletions client/network/src/protocol/generic_proto/handler/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ pub enum NotifsHandlerOut {
/// Received a message on a custom protocol substream.
Notification {
/// Name of the protocol of the message.
protocol_name: Cow<'static, [u8]>,
protocol_name: Cow<'static, str>,

/// Message that has been received.
message: BytesMut,
Expand Down Expand Up @@ -270,7 +270,7 @@ enum NotificationsSinkMessage {
/// Message emitted by [`NotificationsSink::reserve_notification`] and
/// [`NotificationsSink::write_notification_now`].
Notification {
protocol_name: Vec<u8>,
protocol_name: Cow<'static, str>,
encoded_fallback_message: Vec<u8>,
message: Vec<u8>,
},
Expand Down Expand Up @@ -311,13 +311,13 @@ impl NotificationsSink {
/// This method will be removed in a future version.
pub fn send_sync_notification<'a>(
&'a self,
protocol_name: &[u8],
protocol_name: Cow<'static, str>,
encoded_fallback_message: impl Into<Vec<u8>>,
message: impl Into<Vec<u8>>
) {
let mut lock = self.inner.sync_channel.lock();
let result = lock.try_send(NotificationsSinkMessage::Notification {
protocol_name: protocol_name.to_owned(),
protocol_name: protocol_name,
encoded_fallback_message: encoded_fallback_message.into(),
message: message.into()
});
Expand All @@ -336,12 +336,12 @@ impl NotificationsSink {
///
/// The protocol name is expected to be checked ahead of calling this method. It is a logic
/// error to send a notification using an unknown protocol.
pub async fn reserve_notification<'a>(&'a self, protocol_name: &[u8]) -> Result<Ready<'a>, ()> {
pub async fn reserve_notification<'a>(&'a self, protocol_name: Cow<'static, str>) -> Result<Ready<'a>, ()> {
let mut lock = self.inner.async_channel.lock().await;

let poll_ready = future::poll_fn(|cx| lock.poll_ready(cx)).await;
if poll_ready.is_ok() {
Ok(Ready { protocol_name: protocol_name.to_owned(), lock })
Ok(Ready { protocol_name: protocol_name, lock })
} else {
Err(())
}
Expand All @@ -355,7 +355,7 @@ pub struct Ready<'a> {
/// Guarded channel. The channel inside is guaranteed to not be full.
lock: FuturesMutexGuard<'a, mpsc::Sender<NotificationsSinkMessage>>,
/// Name of the protocol. Should match one of the protocols passed at initialization.
protocol_name: Vec<u8>,
protocol_name: Cow<'static, str>,
}

impl<'a> Ready<'a> {
Expand Down Expand Up @@ -392,7 +392,7 @@ impl NotifsHandlerProto {
/// ourselves or respond to handshake from the remote.
pub fn new(
legacy: RegisteredProtocol,
list: impl Into<Vec<(Cow<'static, [u8]>, Arc<RwLock<Vec<u8>>>)>>,
list: impl Into<Vec<(Cow<'static, str>, Arc<RwLock<Vec<u8>>>)>>,
) -> Self {
let list = list.into();

Expand Down Expand Up @@ -613,7 +613,7 @@ impl ProtocolsHandler for NotifsHandler {
message
} => {
for (handler, _) in &mut self.out_handlers {
if handler.protocol_name() == &protocol_name[..] && handler.is_open() {
if *handler.protocol_name() == protocol_name && handler.is_open() {
handler.send_or_discard(message);
continue 'poll_notifs_sink;
}
Expand Down Expand Up @@ -698,7 +698,7 @@ impl ProtocolsHandler for NotifsHandler {
if self.notifications_sink_rx.is_some() {
let msg = NotifsHandlerOut::Notification {
message,
protocol_name: handler.protocol_name().to_owned().into(),
protocol_name: handler.protocol_name().clone(),
};
return Poll::Ready(ProtocolsHandlerEvent::Custom(msg));
}
Expand Down
4 changes: 2 additions & 2 deletions client/network/src/protocol/generic_proto/handler/notif_in.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub enum NotifsInHandlerOut {
impl NotifsInHandlerProto {
/// Builds a new `NotifsInHandlerProto`.
pub fn new(
protocol_name: impl Into<Cow<'static, [u8]>>
protocol_name: impl Into<Cow<'static, str>>
) -> Self {
NotifsInHandlerProto {
in_protocol: NotificationsIn::new(protocol_name),
Expand All @@ -136,7 +136,7 @@ impl IntoProtocolsHandler for NotifsInHandlerProto {

impl NotifsInHandler {
/// Returns the name of the protocol that we accept.
pub fn protocol_name(&self) -> &[u8] {
pub fn protocol_name(&self) -> &Cow<'static, str> {
self.in_protocol.protocol_name()
}
}
Expand Down
Loading