Enforce that ProtocolId is a string (#6953)

* Enforce that ProtocolId is a string

* Fix test
This commit is contained in:
Pierre Krieger
2020-08-26 14:27:30 +02:00
committed by GitHub
parent 8856be80bc
commit 1bd6082cf7
14 changed files with 61 additions and 57 deletions
+9 -9
View File
@@ -80,7 +80,7 @@ pub enum BehaviourOut<B: BlockT> {
/// Peer which sent us a request.
peer: PeerId,
/// Protocol name of the request.
protocol: Vec<u8>,
protocol: String,
/// Time it took to build the response.
build_time: Duration,
},
@@ -88,14 +88,14 @@ pub enum BehaviourOut<B: BlockT> {
RequestStarted {
peer: PeerId,
/// Protocol name of the request.
protocol: Vec<u8>,
protocol: String,
},
/// Finished, successfully or not, a previously-started request.
RequestFinished {
/// Who we were requesting.
peer: PeerId,
/// Protocol name of the request.
protocol: Vec<u8>,
protocol: String,
/// How long before the response came or the request got cancelled.
request_duration: Duration,
},
@@ -300,18 +300,18 @@ Behaviour<B, H> {
block_requests::SendRequestOutcome::Ok => {
self.events.push_back(BehaviourOut::RequestStarted {
peer: target,
protocol: self.block_requests.protocol_name().to_vec(),
protocol: self.block_requests.protocol_name().to_owned(),
});
},
block_requests::SendRequestOutcome::Replaced { request_duration, .. } => {
self.events.push_back(BehaviourOut::RequestFinished {
peer: target.clone(),
protocol: self.block_requests.protocol_name().to_vec(),
protocol: self.block_requests.protocol_name().to_owned(),
request_duration,
});
self.events.push_back(BehaviourOut::RequestStarted {
peer: target,
protocol: self.block_requests.protocol_name().to_vec(),
protocol: self.block_requests.protocol_name().to_owned(),
});
}
block_requests::SendRequestOutcome::NotConnected |
@@ -364,14 +364,14 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviourEventProcess<block_requests::Event<B
block_requests::Event::AnsweredRequest { peer, total_handling_time } => {
self.events.push_back(BehaviourOut::AnsweredRequest {
peer,
protocol: self.block_requests.protocol_name().to_vec(),
protocol: self.block_requests.protocol_name().to_owned(),
build_time: total_handling_time,
});
},
block_requests::Event::Response { peer, original_request: _, response, request_duration } => {
self.events.push_back(BehaviourOut::RequestFinished {
peer: peer.clone(),
protocol: self.block_requests.protocol_name().to_vec(),
protocol: self.block_requests.protocol_name().to_owned(),
request_duration,
});
let ev = self.substrate.on_block_response(peer, response);
@@ -383,7 +383,7 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviourEventProcess<block_requests::Event<B
// we process them by disconnecting the node.
self.events.push_back(BehaviourOut::RequestFinished {
peer: peer.clone(),
protocol: self.block_requests.protocol_name().to_vec(),
protocol: self.block_requests.protocol_name().to_owned(),
request_duration,
});
self.substrate.on_block_request_failed(&peer);
+10 -10
View File
@@ -124,7 +124,7 @@ pub struct Config {
max_response_len: usize,
inactivity_timeout: Duration,
request_timeout: Duration,
protocol: Bytes,
protocol: String,
}
impl Config {
@@ -143,7 +143,7 @@ impl Config {
max_response_len: 16 * 1024 * 1024,
inactivity_timeout: Duration::from_secs(15),
request_timeout: Duration::from_secs(40),
protocol: Bytes::new(),
protocol: String::new(),
};
c.set_protocol(id);
c
@@ -184,11 +184,11 @@ impl Config {
/// Set protocol to use for upgrade negotiation.
pub fn set_protocol(&mut self, id: &ProtocolId) -> &mut Self {
let mut v = Vec::new();
v.extend_from_slice(b"/");
v.extend_from_slice(id.as_bytes());
v.extend_from_slice(b"/sync/2");
self.protocol = v.into();
let mut s = String::new();
s.push_str("/");
s.push_str(id.as_ref());
s.push_str("/sync/2");
self.protocol = s;
self
}
}
@@ -258,7 +258,7 @@ where
}
/// Returns the libp2p protocol name used on the wire (e.g. `/foo/sync/2`).
pub fn protocol_name(&self) -> &[u8] {
pub fn protocol_name(&self) -> &str {
&self.config.protocol
}
@@ -322,7 +322,7 @@ where
request: buf,
original_request: req,
max_response_size: self.config.max_response_len,
protocol: self.config.protocol.clone(),
protocol: self.config.protocol.as_bytes().to_vec().into(),
},
});
@@ -472,7 +472,7 @@ where
fn new_handler(&mut self) -> Self::ProtocolsHandler {
let p = InboundProtocol {
max_request_len: self.config.max_request_len,
protocol: self.config.protocol.clone(),
protocol: self.config.protocol.as_bytes().to_owned().into(),
marker: PhantomData,
};
let mut cfg = OneShotHandlerConfig::default();
+16 -9
View File
@@ -48,6 +48,7 @@ use std::{
io::{self, Write},
net::Ipv4Addr,
path::{Path, PathBuf},
str,
sync::Arc,
};
use zeroize::Zeroize;
@@ -233,20 +234,26 @@ impl<H: ExHashT + Default, B: BlockT> TransactionPool<H, B> for EmptyTransaction
fn transaction(&self, _h: &H) -> Option<B::Extrinsic> { None }
}
/// Name of a protocol, transmitted on the wire. Should be unique for each chain.
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
/// Name of a protocol, transmitted on the wire. Should be unique for each chain. Always UTF-8.
#[derive(Clone, PartialEq, Eq, Hash)]
pub struct ProtocolId(smallvec::SmallVec<[u8; 6]>);
impl<'a> From<&'a [u8]> for ProtocolId {
fn from(bytes: &'a [u8]) -> ProtocolId {
ProtocolId(bytes.into())
impl<'a> From<&'a str> for ProtocolId {
fn from(bytes: &'a str) -> ProtocolId {
ProtocolId(bytes.as_bytes().into())
}
}
impl ProtocolId {
/// Exposes the `ProtocolId` as bytes.
pub fn as_bytes(&self) -> &[u8] {
self.0.as_ref()
impl AsRef<str> for ProtocolId {
fn as_ref(&self) -> &str {
str::from_utf8(&self.0[..])
.expect("the only way to build a ProtocolId is through a UTF-8 String; qed")
}
}
impl fmt::Debug for ProtocolId {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(self.as_ref(), f)
}
}
+6 -6
View File
@@ -752,7 +752,7 @@ impl NetworkBehaviour for DiscoveryBehaviour {
// `DiscoveryBehaviour::new_handler` is still correct.
fn protocol_name_from_protocol_id(id: &ProtocolId) -> Vec<u8> {
let mut v = vec![b'/'];
v.extend_from_slice(id.as_bytes());
v.extend_from_slice(id.as_ref().as_bytes());
v.extend_from_slice(b"/kad");
v
}
@@ -773,7 +773,7 @@ mod tests {
#[test]
fn discovery_working() {
let mut first_swarm_peer_id_and_addr = None;
let protocol_id = ProtocolId::from(b"dot".as_ref());
let protocol_id = ProtocolId::from("dot");
// Build swarms whose behaviour is `DiscoveryBehaviour`, each aware of
// the first swarm via `with_user_defined`.
@@ -877,8 +877,8 @@ mod tests {
#[test]
fn discovery_ignores_peers_with_unknown_protocols() {
let supported_protocol_id = ProtocolId::from(b"a".as_ref());
let unsupported_protocol_id = ProtocolId::from(b"b".as_ref());
let supported_protocol_id = ProtocolId::from("a");
let unsupported_protocol_id = ProtocolId::from("b");
let mut discovery = {
let keypair = Keypair::generate_ed25519();
@@ -929,8 +929,8 @@ mod tests {
#[test]
fn discovery_adds_peer_to_kademlia_of_same_protocol_only() {
let protocol_a = ProtocolId::from(b"a".as_ref());
let protocol_b = ProtocolId::from(b"b".as_ref());
let protocol_a = ProtocolId::from("a");
let protocol_b = ProtocolId::from("b");
let mut discovery = {
let keypair = Keypair::generate_ed25519();
@@ -129,7 +129,7 @@ impl Config {
pub fn set_protocol(&mut self, id: &ProtocolId) -> &mut Self {
let mut v = Vec::new();
v.extend_from_slice(b"/");
v.extend_from_slice(id.as_bytes());
v.extend_from_slice(id.as_ref().as_bytes());
v.extend_from_slice(b"/finality-proof/1");
self.protocol = v.into();
self
+1 -1
View File
@@ -100,7 +100,7 @@ fn build_test_full_node(config: config::NetworkConfiguration)
finality_proof_request_builder: None,
on_demand: None,
transaction_pool: Arc::new(crate::config::EmptyTransactionPool),
protocol_id: config::ProtocolId::from(&b"/test-protocol-name"[..]),
protocol_id: config::ProtocolId::from("/test-protocol-name"),
import_queue,
block_announce_validator: Box::new(
sp_consensus::block_validation::DefaultBlockAnnounceValidator,
@@ -156,13 +156,13 @@ impl Config {
pub fn set_protocol(&mut self, id: &ProtocolId) -> &mut Self {
let mut vl = Vec::new();
vl.extend_from_slice(b"/");
vl.extend_from_slice(id.as_bytes());
vl.extend_from_slice(id.as_ref().as_bytes());
vl.extend_from_slice(b"/light/2");
self.light_protocol = vl.into();
let mut vb = Vec::new();
vb.extend_from_slice(b"/");
vb.extend_from_slice(id.as_bytes());
vb.extend_from_slice(id.as_ref().as_bytes());
vb.extend_from_slice(b"/sync/2");
self.block_protocol = vb.into();
@@ -1447,7 +1447,7 @@ mod tests {
}
fn make_config() -> super::Config {
super::Config::new(&ProtocolId::from(&b"foo"[..]))
super::Config::new(&ProtocolId::from("foo"))
}
fn dummy_header() -> sp_test_primitives::Header {
+2 -2
View File
@@ -419,7 +419,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
let transactions_protocol: Cow<'static, [u8]> = Cow::from({
let mut proto = b"/".to_vec();
proto.extend(protocol_id.as_bytes());
proto.extend(protocol_id.as_ref().as_bytes());
proto.extend(b"/transactions/1");
proto
});
@@ -428,7 +428,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
let block_announces_protocol: Cow<'static, [u8]> = Cow::from({
let mut proto = b"/".to_vec();
proto.extend(protocol_id.as_bytes());
proto.extend(protocol_id.as_ref().as_bytes());
proto.extend(b"/block-announces/1");
proto
});
@@ -83,7 +83,7 @@ fn build_nodes() -> (Swarm<CustomProtoWithAddr>, Swarm<CustomProtoWithAddr>) {
});
let behaviour = CustomProtoWithAddr {
inner: GenericProto::new(local_peer_id, &b"test"[..], &[1], vec![], peerset),
inner: GenericProto::new(local_peer_id, "test", &[1], vec![], peerset),
addrs: addrs
.iter()
.enumerate()
@@ -49,7 +49,7 @@ impl RegisteredProtocol {
-> Self {
let protocol = protocol.into();
let mut base_name = b"/substrate/".to_vec();
base_name.extend_from_slice(protocol.as_bytes());
base_name.extend_from_slice(protocol.as_ref().as_bytes());
base_name.extend_from_slice(b"/");
RegisteredProtocol {
+7 -10
View File
@@ -1497,28 +1497,28 @@ impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> {
Poll::Ready(SwarmEvent::Behaviour(BehaviourOut::AnsweredRequest { protocol, build_time, .. })) => {
if let Some(metrics) = this.metrics.as_ref() {
metrics.requests_in_total
.with_label_values(&[&maybe_utf8_bytes_to_string(&protocol)])
.with_label_values(&[&protocol])
.observe(build_time.as_secs_f64());
}
},
Poll::Ready(SwarmEvent::Behaviour(BehaviourOut::RequestStarted { protocol, .. })) => {
if let Some(metrics) = this.metrics.as_ref() {
metrics.requests_out_started_total
.with_label_values(&[&maybe_utf8_bytes_to_string(&protocol)])
.with_label_values(&[&protocol])
.inc();
}
},
Poll::Ready(SwarmEvent::Behaviour(BehaviourOut::RequestFinished { protocol, request_duration, .. })) => {
if let Some(metrics) = this.metrics.as_ref() {
metrics.requests_out_finished
.with_label_values(&[&maybe_utf8_bytes_to_string(&protocol)])
.with_label_values(&[&protocol])
.observe(request_duration.as_secs_f64());
}
},
Poll::Ready(SwarmEvent::Behaviour(BehaviourOut::RandomKademliaStarted(protocol))) => {
if let Some(metrics) = this.metrics.as_ref() {
metrics.kademlia_random_queries_total
.with_label_values(&[&maybe_utf8_bytes_to_string(protocol.as_bytes())])
.with_label_values(&[&protocol.as_ref()])
.inc();
}
},
@@ -1776,16 +1776,13 @@ impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> {
if let Some(metrics) = this.metrics.as_ref() {
metrics.is_major_syncing.set(is_major_syncing as u64);
for (proto, num_entries) in this.network_service.num_kbuckets_entries() {
let proto = maybe_utf8_bytes_to_string(proto.as_bytes());
metrics.kbuckets_num_nodes.with_label_values(&[&proto]).set(num_entries as u64);
metrics.kbuckets_num_nodes.with_label_values(&[&proto.as_ref()]).set(num_entries as u64);
}
for (proto, num_entries) in this.network_service.num_kademlia_records() {
let proto = maybe_utf8_bytes_to_string(proto.as_bytes());
metrics.kademlia_records_count.with_label_values(&[&proto]).set(num_entries as u64);
metrics.kademlia_records_count.with_label_values(&[&proto.as_ref()]).set(num_entries as u64);
}
for (proto, num_entries) in this.network_service.kademlia_records_total_size() {
let proto = maybe_utf8_bytes_to_string(proto.as_bytes());
metrics.kademlia_records_sizes_total.with_label_values(&[&proto]).set(num_entries as u64);
metrics.kademlia_records_sizes_total.with_label_values(&[&proto.as_ref()]).set(num_entries as u64);
}
metrics.peers_count.set(num_connected_peers as u64);
metrics.peerset_num_discovered.set(this.network_service.user_protocol().num_discovered_peers() as u64);
@@ -101,7 +101,7 @@ fn build_test_full_node(config: config::NetworkConfiguration)
finality_proof_request_builder: None,
on_demand: None,
transaction_pool: Arc::new(crate::config::EmptyTransactionPool),
protocol_id: config::ProtocolId::from(&b"/test-protocol-name"[..]),
protocol_id: config::ProtocolId::from("/test-protocol-name"),
import_queue,
block_announce_validator: Box::new(
sp_consensus::block_validation::DefaultBlockAnnounceValidator,