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

Commit

Permalink
Fix lots of small nits in sc-network (#6028)
Browse files Browse the repository at this point in the history
* Fix lots of small nits in sc-network

* Update client/network/src/protocol/sync/blocks.rs

Co-authored-by: Arkadiy Paronyan <arkady.paronyan@gmail.com>

* Fix warning

* Yes. The line width.

Co-authored-by: Arkadiy Paronyan <arkady.paronyan@gmail.com>
  • Loading branch information
tomaka and arkpar committed May 15, 2020
1 parent d841470 commit f63db92
Show file tree
Hide file tree
Showing 14 changed files with 111 additions and 112 deletions.
4 changes: 2 additions & 2 deletions client/network/src/behaviour.rs
Expand Up @@ -113,7 +113,7 @@ impl<B: BlockT, H: ExHashT> Behaviour<B, H> {
) -> Self {
Behaviour {
substrate,
debug_info: debug_info::DebugInfoBehaviour::new(user_agent, local_public_key.clone()),
debug_info: debug_info::DebugInfoBehaviour::new(user_agent, local_public_key),
discovery: disco_config.finish(),
block_requests,
finality_proof_requests,
Expand Down Expand Up @@ -369,7 +369,7 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviourEventProcess<debug_info::DebugInfoEv
for addr in &info.listen_addrs {
self.discovery.add_self_reported_address(&peer_id, addr.clone());
}
self.substrate.add_discovered_nodes(iter::once(peer_id.clone()));
self.substrate.add_discovered_nodes(iter::once(peer_id));
}
}

Expand Down
4 changes: 2 additions & 2 deletions client/network/src/block_requests.rs
Expand Up @@ -398,9 +398,9 @@ where
break
}

let number = header.number().clone();
let number = *header.number();
let hash = header.hash();
let parent_hash = header.parent_hash().clone();
let parent_hash = *header.parent_hash();
let justification = if get_justification {
self.chain.justification(&BlockId::Hash(hash))?
} else {
Expand Down
2 changes: 1 addition & 1 deletion client/network/src/debug_info.rs
Expand Up @@ -86,7 +86,7 @@ impl DebugInfoBehaviour {
) -> Self {
let identify = {
let proto_version = "/substrate/1.0".to_string();
Identify::new(proto_version, user_agent, local_public_key.clone())
Identify::new(proto_version, user_agent, local_public_key)
};

DebugInfoBehaviour {
Expand Down
2 changes: 1 addition & 1 deletion client/network/src/discovery.rs
Expand Up @@ -675,7 +675,7 @@ impl NetworkBehaviour for DiscoveryBehaviour {
continue;
}

self.discoveries.extend(list.into_iter().map(|(peer_id, _)| peer_id));
self.discoveries.extend(list.map(|(peer_id, _)| peer_id));
if let Some(peer_id) = self.discoveries.pop_front() {
let ev = DiscoveryOut::Discovered(peer_id);
return Poll::Ready(NetworkBehaviourAction::GenerateEvent(ev));
Expand Down
4 changes: 3 additions & 1 deletion client/network/src/light_client_handler.rs
Expand Up @@ -980,7 +980,9 @@ where
let handler = request.connection.map_or(NotifyHandler::Any, NotifyHandler::One);

let request_id = self.next_request_id();
self.peers.get_mut(&peer).map(|p| p.status = PeerStatus::BusyWith(request_id));
if let Some(p) = self.peers.get_mut(&peer) {
p.status = PeerStatus::BusyWith(request_id);
}
self.outstanding.insert(request_id, request);

let event = OutboundProtocol {
Expand Down
2 changes: 1 addition & 1 deletion client/network/src/on_demand_layer.rs
Expand Up @@ -220,7 +220,7 @@ impl<T> Future for RemoteResponse<T> {
fn poll(mut self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> {
match self.receiver.poll_unpin(cx) {
Poll::Ready(Ok(res)) => Poll::Ready(res),
Poll::Ready(Err(_)) => Poll::Ready(Err(From::from(ClientError::RemoteFetchCancelled))),
Poll::Ready(Err(_)) => Poll::Ready(Err(ClientError::RemoteFetchCancelled)),
Poll::Pending => Poll::Pending,
}
}
Expand Down
32 changes: 16 additions & 16 deletions client/network/src/protocol.rs
Expand Up @@ -341,7 +341,7 @@ impl<B: BlockT> BlockAnnouncesHandshake<B> {
let info = chain.info();
BlockAnnouncesHandshake {
genesis_hash: info.genesis_hash,
roles: protocol_config.roles.into(),
roles: protocol_config.roles,
best_number: info.best_number,
best_hash: info.best_hash,
}
Expand Down Expand Up @@ -543,7 +543,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
response: &message::BlockResponse<B>
) -> Option<message::BlockRequest<B>> {
if let Some(ref mut peer) = self.context_data.peers.get_mut(&who) {
if let Some(_) = peer.obsolete_requests.remove(&response.id) {
if peer.obsolete_requests.remove(&response.id).is_some() {
trace!(target: "sync", "Ignoring obsolete block response packet from {} ({})", who, response.id);
return None;
}
Expand Down Expand Up @@ -583,7 +583,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
Ok(message) => message,
Err(err) => {
debug!(target: "sync", "Couldn't decode packet sent by {}: {:?}: {}", who, data, err.what());
self.peerset_handle.report_peer(who.clone(), rep::BAD_MESSAGE);
self.peerset_handle.report_peer(who, rep::BAD_MESSAGE);
return CustomMessageOutcome::None;
}
};
Expand Down Expand Up @@ -633,7 +633,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
GenericMessage::Consensus(msg) =>
return if self.protocol_name_by_engine.contains_key(&msg.engine_id) {
CustomMessageOutcome::NotificationsReceived {
remote: who.clone(),
remote: who,
messages: vec![(msg.engine_id, From::from(msg.data))],
}
} else {
Expand All @@ -655,7 +655,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {

return if !messages.is_empty() {
CustomMessageOutcome::NotificationsReceived {
remote: who.clone(),
remote: who,
messages,
}
} else {
Expand Down Expand Up @@ -713,7 +713,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
self.context_data.peers.remove(&peer)
};
if let Some(_peer_data) = removed {
self.sync.peer_disconnected(peer.clone());
self.sync.peer_disconnected(&peer);

// Notify all the notification protocols as closed.
CustomMessageOutcome::NotificationStreamClosed {
Expand Down Expand Up @@ -774,9 +774,9 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
if blocks.len() >= max {
break;
}
let number = header.number().clone();
let number = *header.number();
let hash = header.hash();
let parent_hash = header.parent_hash().clone();
let parent_hash = *header.parent_hash();
let justification = if get_justification {
self.context_data.chain.justification(&BlockId::Hash(hash)).unwrap_or(None)
} else {
Expand Down Expand Up @@ -875,7 +875,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
return CustomMessageOutcome::None
}

match self.sync.on_block_data(peer, Some(request), response) {
match self.sync.on_block_data(&peer, Some(request), response) {
Ok(sync::OnBlockData::Import(origin, blocks)) =>
CustomMessageOutcome::BlockImport(origin, blocks),
Ok(sync::OnBlockData::Request(peer, req)) => {
Expand Down Expand Up @@ -1329,7 +1329,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
version: CURRENT_VERSION,
min_supported_version: MIN_VERSION,
genesis_hash: info.genesis_hash,
roles: self.config.roles.into(),
roles: self.config.roles,
best_number: info.best_number,
best_hash: info.best_hash,
chain_status: Vec::new(), // TODO: find a way to make this backwards-compatible
Expand All @@ -1355,7 +1355,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
message::BlockState::Normal => false,
};

match self.sync.on_block_announce(who.clone(), hash, &announce, is_their_best) {
match self.sync.on_block_announce(&who, hash, &announce, is_their_best) {
sync::OnBlockAnnounce::Nothing => {
// `on_block_announce` returns `OnBlockAnnounce::ImportHeader`
// when we have all data required to import the block
Expand All @@ -1375,7 +1375,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
// to import header from announced block let's construct response to request that normally would have
// been sent over network (but it is not in our case)
let blocks_to_import = self.sync.on_block_data(
who.clone(),
&who,
None,
message::generic::BlockResponse {
id: 0,
Expand Down Expand Up @@ -1911,7 +1911,7 @@ fn send_request<B: BlockT, H: ExHashT>(
if let GenericMessage::BlockRequest(ref mut r) = message {
if let Some(ref mut peer) = peers.get_mut(who) {
r.id = peer.next_request_id;
peer.next_request_id = peer.next_request_id + 1;
peer.next_request_id += 1;
if let Some((timestamp, request)) = peer.block_request.take() {
trace!(target: "sync", "Request {} for {} is now obsolete.", request.id, who);
peer.obsolete_requests.insert(request.id, timestamp);
Expand Down Expand Up @@ -2002,7 +2002,7 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviour for Protocol<B, H> {
for (id, r) in self.sync.block_requests() {
if self.use_new_block_requests_protocol {
let event = CustomMessageOutcome::BlockRequest {
target: id,
target: id.clone(),
request: r,
};
self.pending_messages.push_back(event);
Expand Down Expand Up @@ -2073,11 +2073,11 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviour for Protocol<B, H> {

let outcome = match event {
GenericProtoOut::CustomProtocolOpen { peer_id, .. } => {
self.on_peer_connected(peer_id.clone());
self.on_peer_connected(peer_id);
CustomMessageOutcome::None
}
GenericProtoOut::CustomProtocolClosed { peer_id, .. } => {
self.on_peer_disconnected(peer_id.clone())
self.on_peer_disconnected(peer_id)
},
GenericProtoOut::LegacyMessage { peer_id, message } =>
self.on_custom_message(peer_id, message),
Expand Down
16 changes: 8 additions & 8 deletions client/network/src/protocol/generic_proto/behaviour.rs
Expand Up @@ -507,7 +507,7 @@ impl GenericProto {
///
/// Can be called multiple times with the same `PeerId`s.
pub fn add_discovered_nodes(&mut self, peer_ids: impl Iterator<Item = PeerId>) {
self.peerset.discovered(peer_ids.into_iter().map(|peer_id| {
self.peerset.discovered(peer_ids.map(|peer_id| {
debug!(target: "sub-libp2p", "PSM <= Discovered({:?})", peer_id);
peer_id
}));
Expand Down Expand Up @@ -616,8 +616,8 @@ impl GenericProto {
debug!(target: "sub-libp2p", "PSM => Connect({:?}): Will start to connect at \
until {:?}", occ_entry.key(), until);
*occ_entry.into_mut() = PeerState::PendingRequest {
timer: futures_timer::Delay::new(until.clone() - now),
timer_deadline: until.clone(),
timer: futures_timer::Delay::new(*until - now),
timer_deadline: *until,
};
},

Expand All @@ -639,8 +639,8 @@ impl GenericProto {
occ_entry.key(), banned);
*occ_entry.into_mut() = PeerState::DisabledPendingEnable {
open,
timer: futures_timer::Delay::new(banned.clone() - now),
timer_deadline: banned.clone(),
timer: futures_timer::Delay::new(*banned - now),
timer_deadline: *banned,
};
},

Expand Down Expand Up @@ -879,7 +879,7 @@ impl NetworkBehaviour for GenericProto {
// this peer", and not "banned" in the sense that we would refuse the peer altogether.
(st @ &mut PeerState::Poisoned, endpoint @ ConnectedPoint::Listener { .. }) |
(st @ &mut PeerState::Banned { .. }, endpoint @ ConnectedPoint::Listener { .. }) => {
let incoming_id = self.next_incoming_index.clone();
let incoming_id = self.next_incoming_index;
self.next_incoming_index.0 = match self.next_incoming_index.0.checked_add(1) {
Some(v) => v,
None => {
Expand Down Expand Up @@ -1200,7 +1200,7 @@ impl NetworkBehaviour for GenericProto {
debug!(target: "sub-libp2p", "External API <= Closed({:?})", source);
let event = GenericProtoOut::CustomProtocolClosed {
reason,
peer_id: source.clone(),
peer_id: source,
};
self.events.push(NetworkBehaviourAction::GenerateEvent(event));
} else {
Expand Down Expand Up @@ -1384,7 +1384,7 @@ impl NetworkBehaviour for GenericProto {
*peer_state = PeerState::Enabled { open };
}

st @ _ => *peer_state = st,
st => *peer_state = st,
}
}

Expand Down
18 changes: 9 additions & 9 deletions client/network/src/protocol/generic_proto/handler/group.rs
Expand Up @@ -483,35 +483,35 @@ impl ProtocolsHandler for NotifsHandler {
) -> Poll<
ProtocolsHandlerEvent<Self::OutboundProtocol, Self::OutboundOpenInfo, Self::OutEvent, Self::Error>
> {
while let Poll::Ready(ev) = self.legacy.poll(cx) {
match ev {
if let Poll::Ready(ev) = self.legacy.poll(cx) {
return match ev {
ProtocolsHandlerEvent::OutboundSubstreamRequest { protocol, info: () } =>
return Poll::Ready(ProtocolsHandlerEvent::OutboundSubstreamRequest {
Poll::Ready(ProtocolsHandlerEvent::OutboundSubstreamRequest {
protocol: protocol.map_upgrade(EitherUpgrade::B),
info: None,
}),
ProtocolsHandlerEvent::Custom(LegacyProtoHandlerOut::CustomProtocolOpen { endpoint, .. }) =>
return Poll::Ready(ProtocolsHandlerEvent::Custom(
Poll::Ready(ProtocolsHandlerEvent::Custom(
NotifsHandlerOut::Open { endpoint }
)),
ProtocolsHandlerEvent::Custom(LegacyProtoHandlerOut::CustomProtocolClosed { endpoint, reason }) =>
return Poll::Ready(ProtocolsHandlerEvent::Custom(
Poll::Ready(ProtocolsHandlerEvent::Custom(
NotifsHandlerOut::Closed { endpoint, reason }
)),
ProtocolsHandlerEvent::Custom(LegacyProtoHandlerOut::CustomMessage { message }) =>
return Poll::Ready(ProtocolsHandlerEvent::Custom(
Poll::Ready(ProtocolsHandlerEvent::Custom(
NotifsHandlerOut::CustomMessage { message }
)),
ProtocolsHandlerEvent::Custom(LegacyProtoHandlerOut::Clogged { messages }) =>
return Poll::Ready(ProtocolsHandlerEvent::Custom(
Poll::Ready(ProtocolsHandlerEvent::Custom(
NotifsHandlerOut::Clogged { messages }
)),
ProtocolsHandlerEvent::Custom(LegacyProtoHandlerOut::ProtocolError { is_severe, error }) =>
return Poll::Ready(ProtocolsHandlerEvent::Custom(
Poll::Ready(ProtocolsHandlerEvent::Custom(
NotifsHandlerOut::ProtocolError { is_severe, error }
)),
ProtocolsHandlerEvent::Close(err) =>
return Poll::Ready(ProtocolsHandlerEvent::Close(EitherError::B(err))),
Poll::Ready(ProtocolsHandlerEvent::Close(EitherError::B(err))),
}
}

Expand Down
Expand Up @@ -390,8 +390,8 @@ pub enum NotificationsOutError {
/// Remote doesn't process our messages quickly enough.
///
/// > **Note**: This is not necessarily the remote's fault, and could also be caused by the
/// > local node sending data too quickly. Properly doing back-pressure, however,
/// > would require a deep refactoring effort in Substrate as a whole.
/// > local node sending data too quickly. Properly doing back-pressure, however,
/// > would require a deep refactoring effort in Substrate as a whole.
Clogged,
}

Expand Down

0 comments on commit f63db92

Please sign in to comment.