Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
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
Prev Previous commit
Next Next commit
Special case first protocol as the one bearing the handshake
  • Loading branch information
tomaka committed Sep 14, 2020
commit 776ee3b2598d83e1ec0d5dfa1dc65ab79a5fee9a
3 changes: 2 additions & 1 deletion client/network/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,9 +419,10 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
versions,
build_status_message(&config, &chain),
peerset,
// As documented in `GenericProto`, the first protocol in the list is always the
// one carrying the handshake reported in the `CustomProtocolOpen` event.
iter::once((block_announces_protocol.clone(), block_announces_handshake))
.chain(iter::once((transactions_protocol.clone(), vec![]))),
0,
)
};

Expand Down
8 changes: 0 additions & 8 deletions client/network/src/protocol/generic_proto/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,6 @@ pub struct GenericProto {
/// initial handshake.
notif_protocols: Vec<(Cow<'static, str>, Arc<RwLock<Vec<u8>>>)>,

/// Index within `notif_protocols` of the protocol containing the handshake to report on the
/// external API.
handshake_protocol_index: usize,

/// Receiver for instructions about who to connect to or disconnect from.
peerset: sc_peerset::Peerset,

Expand Down Expand Up @@ -341,14 +337,12 @@ impl GenericProto {
handshake_message: Vec<u8>,
peerset: sc_peerset::Peerset,
notif_protocols: impl Iterator<Item = (Cow<'static, str>, Vec<u8>)>,
handshake_protocol_index: usize,
) -> Self {
let notif_protocols = notif_protocols
.map(|(n, hs)| (n, Arc::new(RwLock::new(hs))))
.collect::<Vec<_>>();

assert!(!notif_protocols.is_empty());
assert!(handshake_protocol_index < notif_protocols.len());

let legacy_handshake_message = Arc::new(RwLock::new(handshake_message));
let legacy_protocol = RegisteredProtocol::new(protocol, versions, legacy_handshake_message);
Expand All @@ -357,7 +351,6 @@ impl GenericProto {
local_peer_id,
legacy_protocol,
notif_protocols,
handshake_protocol_index,
peerset,
peers: FnvHashMap::default(),
delays: Default::default(),
Expand Down Expand Up @@ -869,7 +862,6 @@ impl NetworkBehaviour for GenericProto {
NotifsHandlerProto::new(
self.legacy_protocol.clone(),
self.notif_protocols.clone(),
self.handshake_protocol_index,
)
}

Expand Down
27 changes: 6 additions & 21 deletions client/network/src/protocol/generic_proto/handler/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,6 @@ pub struct NotifsHandlerProto {
/// Prototypes for handlers for outbound substreams, and the initial handshake message we send.
out_handlers: Vec<(NotifsOutHandlerProto, Arc<RwLock<Vec<u8>>>)>,

/// Index within `in_handlers` and `out_handlers` of the protocol containing the handshake to
/// report on the external API.
handshake_protocol_index: usize,

/// Prototype for handler for backwards-compatibility.
legacy: LegacyProtoHandlerProto,
}
Expand All @@ -111,10 +107,6 @@ pub struct NotifsHandler {
/// Handlers for outbound substreams, and the initial handshake message we send.
out_handlers: Vec<(NotifsOutHandler, Arc<RwLock<Vec<u8>>>)>,

/// Index within `in_handlers` and `out_handlers` of the protocol containing the handshake to
/// report on the external API.
handshake_protocol_index: usize,

/// Whether we are the connection dialer or listener.
endpoint: ConnectedPoint,

Expand Down Expand Up @@ -179,7 +171,6 @@ impl IntoProtocolsHandler for NotifsHandlerProto {
.into_iter()
.map(|(proto, msg)| (proto.into_handler(remote_peer_id, connected_point), msg))
.collect(),
handshake_protocol_index: self.handshake_protocol_index,
endpoint: connected_point.clone(),
legacy: self.legacy.into_handler(remote_peer_id, connected_point),
pending_handshake: None,
Expand Down Expand Up @@ -371,23 +362,19 @@ impl NotifsHandlerProto {
/// handshake. At the moment, the message is always the same whether we open a substream
/// ourselves or respond to handshake from the remote.
///
/// `handshake_protocol_index` is the index, within `list`, of the protocol that contains
/// the handshake to report through the [`NotifsHandlerOut::Open`] event.
/// The first protocol in `list` is special-cased as the protocol that contains the handshake
/// to report through the [`NotifsHandlerOut::Open`] event.
///
/// # Panic
///
/// - Panics if `list` is empty (as `handshake_protocol_index` can't possibly be correct).
/// - Panics if `handshake_protocol_index` is >= `list.len()`.
/// - Panics if `list` is empty.
///
pub fn new(
legacy: RegisteredProtocol,
list: impl Into<Vec<(Cow<'static, str>, Arc<RwLock<Vec<u8>>>)>>,
handshake_protocol_index: usize,
) -> Self {
let list = list.into();

assert!(!list.is_empty());
assert!(handshake_protocol_index < list.len());

let out_handlers = list
.clone()
Expand All @@ -404,7 +391,6 @@ impl NotifsHandlerProto {
NotifsHandlerProto {
in_handlers,
out_handlers,
handshake_protocol_index,
legacy: LegacyProtoHandlerProto::new(legacy),
}
}
Expand Down Expand Up @@ -637,9 +623,8 @@ impl ProtocolsHandler for NotifsHandler {
}

// If `self.pending_handshake` is `Some`, we are in a state where the handshake-bearing
// substream (either the legacy substream of the one indicated by
// `handshake_protocol_index`) is open but the user isn't aware yet of the substreams
// being open.
// substream (either the legacy substream or the one special-cased as providing the
// handshake) is open but the user isn't aware yet of the substreams being open.
// When that is the case, neither the legacy substream nor the incoming notifications
// substreams should be polled, otherwise there is a risk of receiving messages from them.
if self.pending_handshake.is_none() {
Expand Down Expand Up @@ -741,7 +726,7 @@ impl ProtocolsHandler for NotifsHandler {

// Opened substream on the handshake-bearing notification protocol.
ProtocolsHandlerEvent::Custom(NotifsOutHandlerOut::Open { handshake })
if handler_num == self.handshake_protocol_index =>
if handler_num == 0 =>
{
if self.notifications_sink_rx.is_none() && self.pending_handshake.is_none() {
self.pending_handshake = Some(handshake);
Expand Down
2 changes: 1 addition & 1 deletion client/network/src/protocol/generic_proto/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ fn build_nodes() -> (Swarm<CustomProtoWithAddr>, Swarm<CustomProtoWithAddr>) {
let behaviour = CustomProtoWithAddr {
inner: GenericProto::new(
local_peer_id, "test", &[1], vec![], peerset,
iter::once(("/foo".into(), Vec::new())), 0
iter::once(("/foo".into(), Vec::new()))
),
addrs: addrs
.iter()
Expand Down