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

No longer actively open legacy substreams #7076

Merged
22 commits merged into from
Oct 16, 2020
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
bf458b3
Allow remotes to not open a legacy substream
tomaka Sep 10, 2020
a009846
No longer actively open legacy substreams
tomaka Sep 10, 2020
d7a506d
Misc fixes
tomaka Sep 10, 2020
f74cc8f
Merge branch 'allow-no-legacy' into no-longer-open-substream
tomaka Sep 10, 2020
5555ae4
Line width
tomaka Sep 10, 2020
776ee3b
Special case first protocol as the one bearing the handshake
tomaka Sep 14, 2020
3608b98
Merge remote-tracking branch 'upstream/master' into allow-no-legacy
tomaka Sep 14, 2020
1f98436
Merge branch 'allow-no-legacy' into no-longer-open-substream
tomaka Sep 14, 2020
905ba6f
Merge remote-tracking branch 'upstream/master' into no-longer-open-su…
tomaka Sep 15, 2020
2c7e176
Legacy opening state no longer keeps connection alive
tomaka Sep 15, 2020
c40d674
Remove now-unused code
tomaka Sep 15, 2020
052b8d8
Simplify inject_dial_upgrade_error
tomaka Sep 15, 2020
3d227d6
Merge remote-tracking branch 'upstream/master' into no-longer-open-su…
tomaka Sep 17, 2020
994a274
Merge remote-tracking branch 'upstream/master' into no-longer-open-su…
tomaka Sep 21, 2020
5d3d613
[chaos:basic]
tomaka Sep 21, 2020
f48d45e
Merge remote-tracking branch 'upstream/master' into no-longer-open-su…
tomaka Sep 21, 2020
8d12d57
Merge remote-tracking branch 'upstream/master' into no-longer-open-su…
tomaka Sep 21, 2020
e4f6ed3
[chaos:basic]
tomaka Sep 21, 2020
5af3ea5
[chaos:basic]
tomaka Sep 21, 2020
6055917
Merge remote-tracking branch 'upstream/master' into no-longer-open-su…
tomaka Sep 23, 2020
102a834
Merge remote-tracking branch 'upstream/master' into no-longer-open-su…
tomaka Sep 25, 2020
58be34f
Merge remote-tracking branch 'upstream/master' into no-longer-open-su…
tomaka Oct 12, 2020
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
21 changes: 0 additions & 21 deletions client/network/src/protocol/generic_proto/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1368,27 +1368,6 @@ impl NetworkBehaviour for GenericProto {

self.events.push_back(NetworkBehaviourAction::GenerateEvent(event));
}

// Don't do anything for non-severe errors except report them.
NotifsHandlerOut::ProtocolError { is_severe, ref error } if !is_severe => {
debug!(target: "sub-libp2p", "Handler({:?}) => Benign protocol error: {:?}",
source, error)
}

NotifsHandlerOut::ProtocolError { error, .. } => {
debug!(target: "sub-libp2p",
"Handler({:?}) => Severe protocol error: {:?}",
source, error);
// A severe protocol error happens when we detect a "bad" peer, such as a peer on
// a different chain, or a peer that doesn't speak the same protocol(s). We
// decrease the peer's reputation, hence lowering the chances we try this peer
// again in the short term.
self.peerset.report_peer(
source.clone(),
sc_peerset::ReputationChange::new(i32::min_value(), "Protocol error")
);
self.disconnect_peer_inner(&source, Some(Duration::from_secs(5)));
}
}
}

Expand Down
70 changes: 16 additions & 54 deletions client/network/src/protocol/generic_proto/handler/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ use crate::protocol::generic_proto::{
};

use bytes::BytesMut;
use libp2p::core::{either::{EitherError, EitherOutput}, ConnectedPoint, PeerId};
use libp2p::core::upgrade::{EitherUpgrade, UpgradeError, SelectUpgrade, InboundUpgrade, OutboundUpgrade};
use libp2p::core::{either::EitherOutput, ConnectedPoint, PeerId};
use libp2p::core::upgrade::{UpgradeError, SelectUpgrade, InboundUpgrade, OutboundUpgrade};
use libp2p::swarm::{
ProtocolsHandler, ProtocolsHandlerEvent,
IntoProtocolsHandler,
Expand All @@ -70,7 +70,7 @@ use futures::{
};
use log::{debug, error};
use parking_lot::{Mutex, RwLock};
use std::{borrow::Cow, error, io, str, sync::Arc, task::{Context, Poll}};
use std::{borrow::Cow, str, sync::Arc, task::{Context, Poll}};

/// Number of pending notifications in asynchronous contexts.
/// See [`NotificationsSink::reserve_notification`] for context.
Expand Down Expand Up @@ -230,14 +230,6 @@ pub enum NotifsHandlerOut {
/// Message that has been received.
message: BytesMut,
},

/// An error has happened on the protocol level with this node.
ProtocolError {
/// If true the error is severe, such as a protocol violation.
is_severe: bool,
/// The error that happened.
error: Box<dyn error::Error + Send + Sync>,
},
}

/// Sink connected directly to the node background task. Allows sending notifications to the peer.
Expand Down Expand Up @@ -401,9 +393,9 @@ impl ProtocolsHandler for NotifsHandler {
type OutEvent = NotifsHandlerOut;
type Error = NotifsHandlerError;
type InboundProtocol = SelectUpgrade<UpgradeCollec<NotificationsIn>, RegisteredProtocol>;
type OutboundProtocol = EitherUpgrade<NotificationsOut, RegisteredProtocol>;
// Index within the `out_handlers`; None for legacy
type OutboundOpenInfo = Option<usize>;
type OutboundProtocol = NotificationsOut;
// Index within the `out_handlers`
type OutboundOpenInfo = usize;
type InboundOpenInfo = ();

fn listen_protocol(&self) -> SubstreamProtocol<Self::InboundProtocol, ()> {
Expand Down Expand Up @@ -433,13 +425,7 @@ impl ProtocolsHandler for NotifsHandler {
out: <Self::OutboundProtocol as OutboundUpgrade<NegotiatedSubstream>>::Output,
num: Self::OutboundOpenInfo
) {
match (out, num) {
(EitherOutput::First(out), Some(num)) =>
self.out_handlers[num].0.inject_fully_negotiated_outbound(out, ()),
(EitherOutput::Second(out), None) =>
self.legacy.inject_fully_negotiated_outbound(out, ()),
_ => error!("inject_fully_negotiated_outbound called with wrong parameters"),
}
self.out_handlers[num].0.inject_fully_negotiated_outbound(out, ())
}

fn inject_event(&mut self, message: NotifsHandlerIn) {
Expand Down Expand Up @@ -488,45 +474,30 @@ impl ProtocolsHandler for NotifsHandler {

fn inject_dial_upgrade_error(
&mut self,
num: Option<usize>,
err: ProtocolsHandlerUpgrErr<EitherError<NotificationsHandshakeError, io::Error>>
num: usize,
err: ProtocolsHandlerUpgrErr<NotificationsHandshakeError>
) {
match (err, num) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it still makes sense to match on num, right?

(ProtocolsHandlerUpgrErr::Timeout, Some(num)) =>
(ProtocolsHandlerUpgrErr::Timeout, num) =>
self.out_handlers[num].0.inject_dial_upgrade_error(
(),
ProtocolsHandlerUpgrErr::Timeout
),
(ProtocolsHandlerUpgrErr::Timeout, None) =>
self.legacy.inject_dial_upgrade_error((), ProtocolsHandlerUpgrErr::Timeout),
(ProtocolsHandlerUpgrErr::Timer, Some(num)) =>
(ProtocolsHandlerUpgrErr::Timer, num) =>
self.out_handlers[num].0.inject_dial_upgrade_error(
(),
ProtocolsHandlerUpgrErr::Timer
),
(ProtocolsHandlerUpgrErr::Timer, None) =>
self.legacy.inject_dial_upgrade_error((), ProtocolsHandlerUpgrErr::Timer),
(ProtocolsHandlerUpgrErr::Upgrade(UpgradeError::Select(err)), Some(num)) =>
(ProtocolsHandlerUpgrErr::Upgrade(UpgradeError::Select(err)), num) =>
self.out_handlers[num].0.inject_dial_upgrade_error(
(),
ProtocolsHandlerUpgrErr::Upgrade(UpgradeError::Select(err))
),
(ProtocolsHandlerUpgrErr::Upgrade(UpgradeError::Select(err)), None) =>
self.legacy.inject_dial_upgrade_error(
(),
ProtocolsHandlerUpgrErr::Upgrade(UpgradeError::Select(err))
),
(ProtocolsHandlerUpgrErr::Upgrade(UpgradeError::Apply(EitherError::A(err))), Some(num)) =>
(ProtocolsHandlerUpgrErr::Upgrade(UpgradeError::Apply(err)), num) =>
self.out_handlers[num].0.inject_dial_upgrade_error(
(),
ProtocolsHandlerUpgrErr::Upgrade(UpgradeError::Apply(err))
),
(ProtocolsHandlerUpgrErr::Upgrade(UpgradeError::Apply(EitherError::B(err))), None) =>
self.legacy.inject_dial_upgrade_error(
(),
ProtocolsHandlerUpgrErr::Upgrade(UpgradeError::Apply(err))
),
_ => error!("inject_dial_upgrade_error called with bad parameters"),
}
}

Expand Down Expand Up @@ -632,12 +603,8 @@ impl ProtocolsHandler for NotifsHandler {
if self.pending_handshake.is_none() {
while let Poll::Ready(ev) = self.legacy.poll(cx) {
match ev {
ProtocolsHandlerEvent::OutboundSubstreamRequest { protocol } =>
return Poll::Ready(ProtocolsHandlerEvent::OutboundSubstreamRequest {
protocol: protocol
.map_upgrade(EitherUpgrade::B)
.map_info(|()| None)
}),
ProtocolsHandlerEvent::OutboundSubstreamRequest { protocol, .. } =>
match *protocol.info() {},
ProtocolsHandlerEvent::Custom(LegacyProtoHandlerOut::CustomProtocolOpen {
received_handshake,
..
Expand All @@ -663,10 +630,6 @@ impl ProtocolsHandler for NotifsHandler {
NotifsHandlerOut::CustomMessage { message }
))
},
ProtocolsHandlerEvent::Custom(LegacyProtoHandlerOut::ProtocolError { is_severe, error }) =>
return Poll::Ready(ProtocolsHandlerEvent::Custom(
NotifsHandlerOut::ProtocolError { is_severe, error }
)),
ProtocolsHandlerEvent::Close(err) =>
return Poll::Ready(ProtocolsHandlerEvent::Close(NotifsHandlerError::Legacy(err))),
}
Expand Down Expand Up @@ -723,8 +686,7 @@ impl ProtocolsHandler for NotifsHandler {
ProtocolsHandlerEvent::OutboundSubstreamRequest { protocol } =>
return Poll::Ready(ProtocolsHandlerEvent::OutboundSubstreamRequest {
protocol: protocol
.map_upgrade(EitherUpgrade::A)
.map_info(|()| Some(handler_num))
.map_info(|()| handler_num),
}),
ProtocolsHandlerEvent::Close(err) => void::unreachable(err),

Expand Down
Loading