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 1 commit
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 @@ -218,7 +218,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 @@ -413,7 +413,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>)>,
/// Maximum allowed number of incoming connections.
pub in_peers: u32,
/// Number of outgoing connections we're trying to maintain.
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