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

Commit

Permalink
No longer actively open legacy substreams (#7076)
Browse files Browse the repository at this point in the history 
* Allow remotes to not open a legacy substream

* No longer actively open legacy substreams

* Misc fixes

* Line width

* Special case first protocol as the one bearing the handshake

* Legacy opening state no longer keeps connection alive

* Remove now-unused code

* Simplify inject_dial_upgrade_error

* [chaos:basic]

* [chaos:basic]

* [chaos:basic]
  • Loading branch information
@tomaka
tomaka authored Oct 16, 2020
1 parent bb79f65 commit ec18346
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 172 deletions.
21 changes: 0 additions & 21 deletions client/network/src/protocol/generic_proto/behaviour.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1349,27 +1349,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
72 changes: 17 additions & 55 deletions client/network/src/protocol/generic_proto/handler/group.rs
View file
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) {
(ProtocolsHandlerUpgrErr::Timeout, Some(num)) =>
match err {
ProtocolsHandlerUpgrErr::Timeout =>
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 =>
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)) =>
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)) =>
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

0 comments on commit ec18346

Please sign in to comment.