From edb99eded624f5129e9af7183c3cc814c95cf63c Mon Sep 17 00:00:00 2001 From: Roman Borschel Date: Wed, 18 Nov 2020 12:03:07 +0100 Subject: [PATCH 01/19] [multistream-select] Fix `ls` response encoding/decoding (spec-compliance). (#1811) * Fix ls response encoding/decoding. Thereby remove the now unnecessary arbitrary protocol name length limit. Since it an 'ls' response is always terminated with a dedicated newline (and thus ends with two newlines), an 'ls' response with a single protocol can be disambiguated from a single protocol response by this additional newline. * More commentary * Update versions and changelogs. * Resolve remaining conflict. * Permit empty ls responses, as before. --- CHANGELOG.md | 4 ++ Cargo.toml | 2 +- core/CHANGELOG.md | 2 + core/Cargo.toml | 2 +- misc/multistream-select/CHANGELOG.md | 6 ++ misc/multistream-select/Cargo.toml | 2 +- misc/multistream-select/src/protocol.rs | 75 +++++++++++++------------ 7 files changed, 54 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b56f8cf693b5..1e408bd9d46e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ - [`parity-multiaddr` CHANGELOG](misc/multiaddr/CHANGELOG.md) - [`libp2p-core-derive` CHANGELOG](misc/core-derive/CHANGELOG.md) +# Version 0.31.0 [unreleased] + +- Update `multistream-select` and all dependent crates. + # Version 0.30.1 [2020-11-11] - Update `libp2p-plaintext`. diff --git a/Cargo.toml b/Cargo.toml index 1342ae8c11b6..2c87e40cf61f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "libp2p" edition = "2018" description = "Peer-to-peer networking library" -version = "0.30.1" +version = "0.31.0" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index 837c2221ad51..724c55a0cda4 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -2,6 +2,8 @@ - Update `multihash`. +- Update `multistream-select`. + # 0.24.0 [2020-11-09] - Remove `ConnectionInfo` trait and replace it with `PeerId` diff --git a/core/Cargo.toml b/core/Cargo.toml index 5e8724cd32c1..bd209e2301f3 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -23,7 +23,7 @@ libsecp256k1 = { version = "0.3.1", optional = true } log = "0.4" multiaddr = { package = "parity-multiaddr", version = "0.9.4", path = "../misc/multiaddr" } multihash = { version = "0.13", default-features = false, features = ["std", "multihash-impl", "identity", "sha2"] } -multistream-select = { version = "0.8.5", path = "../misc/multistream-select" } +multistream-select = { version = "0.9.0", path = "../misc/multistream-select" } parking_lot = "0.11.0" pin-project = "1.0.0" prost = "0.6.1" diff --git a/misc/multistream-select/CHANGELOG.md b/misc/multistream-select/CHANGELOG.md index ac24466f6a95..e61fa9b9d686 100644 --- a/misc/multistream-select/CHANGELOG.md +++ b/misc/multistream-select/CHANGELOG.md @@ -1,3 +1,9 @@ +# 0.9.0 [unreleased] + +- Fix the encoding and decoding of `ls` responses to + be spec-compliant and interoperable with other implementations. + For a clean upgrade, `0.8.4` must already be deployed. + # 0.8.5 [2020-11-09] - During negotiation do not interpret EOF error as an IO error, but instead as a diff --git a/misc/multistream-select/Cargo.toml b/misc/multistream-select/Cargo.toml index d8e3fa167af5..7482fc986a15 100644 --- a/misc/multistream-select/Cargo.toml +++ b/misc/multistream-select/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "multistream-select" description = "Multistream-select negotiation protocol for libp2p" -version = "0.8.5" +version = "0.9.0" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/misc/multistream-select/src/protocol.rs b/misc/multistream-select/src/protocol.rs index c15ddbc098d7..5248247f7ce0 100644 --- a/misc/multistream-select/src/protocol.rs +++ b/misc/multistream-select/src/protocol.rs @@ -35,18 +35,6 @@ use unsigned_varint as uvi; /// The maximum number of supported protocols that can be processed. const MAX_PROTOCOLS: usize = 1000; -/// The maximum length (in bytes) of a protocol name. -/// -/// This limit is necessary in order to be able to unambiguously parse -/// response messages without knowledge of the corresponding request. -/// 140 comes about from 3 * 47 = 141, where 47 is the ascii/utf8 -/// encoding of the `/` character and an encoded protocol name is -/// at least 3 bytes long (uvi-length followed by `/` and `\n`). -/// Hence a protocol list response message with 47 protocols is at least -/// 141 bytes long and thus such a response cannot be mistaken for a -/// single protocol response. See `Message::decode`. -const MAX_PROTOCOL_LEN: usize = 140; - /// The encoded form of a multistream-select 1.0.0 header message. const MSG_MULTISTREAM_1_0: &[u8] = b"/multistream/1.0.0\n"; /// The encoded form of a multistream-select 1.0.0 header message. @@ -131,7 +119,7 @@ impl TryFrom for Protocol { type Error = ProtocolError; fn try_from(value: Bytes) -> Result { - if !value.as_ref().starts_with(b"/") || value.len() > MAX_PROTOCOL_LEN { + if !value.as_ref().starts_with(b"/") { return Err(ProtocolError::InvalidProtocol) } Ok(Protocol(value)) @@ -190,7 +178,7 @@ impl Message { let len = p.0.as_ref().len() + 1; // + 1 for \n dest.reserve(len); dest.put(p.0.as_ref()); - dest.put(&b"\n"[..]); + dest.put_u8(b'\n'); Ok(()) } Message::ListProtocols => { @@ -200,14 +188,15 @@ impl Message { } Message::Protocols(ps) => { let mut buf = uvi::encode::usize_buffer(); - let mut out_msg = Vec::from(uvi::encode::usize(ps.len(), &mut buf)); + let mut encoded = Vec::with_capacity(ps.len()); for p in ps { - out_msg.extend(uvi::encode::usize(p.0.as_ref().len() + 1, &mut buf)); // +1 for '\n' - out_msg.extend_from_slice(p.0.as_ref()); - out_msg.push(b'\n') + encoded.extend(uvi::encode::usize(p.0.as_ref().len() + 1, &mut buf)); // +1 for '\n' + encoded.extend_from_slice(p.0.as_ref()); + encoded.push(b'\n') } - dest.reserve(out_msg.len()); - dest.put(out_msg.as_ref()); + encoded.push(b'\n'); + dest.reserve(encoded.len()); + dest.put(encoded.as_ref()); Ok(()) } Message::NotAvailable => { @@ -228,11 +217,6 @@ impl Message { return Ok(Message::Header(Version::V1)) } - if msg.get(0) == Some(&b'/') && msg.last() == Some(&b'\n') && msg.len() <= MAX_PROTOCOL_LEN { - let p = Protocol::try_from(msg.split_to(msg.len() - 1))?; - return Ok(Message::Protocol(p)); - } - if msg == MSG_PROTOCOL_NA { return Ok(Message::NotAvailable); } @@ -241,21 +225,40 @@ impl Message { return Ok(Message::ListProtocols) } - // At this point, it must be a varint number of protocols, i.e. - // a `Protocols` message. - let (num_protocols, mut remaining) = uvi::decode::usize(&msg)?; - if num_protocols > MAX_PROTOCOLS { - return Err(ProtocolError::TooManyProtocols) + // If it starts with a `/`, ends with a line feed without any + // other line feeds in-between, it must be a protocol name. + if msg.get(0) == Some(&b'/') && msg.last() == Some(&b'\n') && + !msg[.. msg.len() - 1].contains(&b'\n') + { + let p = Protocol::try_from(msg.split_to(msg.len() - 1))?; + return Ok(Message::Protocol(p)); } - let mut protocols = Vec::with_capacity(num_protocols); - for _ in 0 .. num_protocols { - let (len, rem) = uvi::decode::usize(remaining)?; - if len == 0 || len > rem.len() || rem[len - 1] != b'\n' { + + // At this point, it must be an `ls` response, i.e. one or more + // length-prefixed, newline-delimited protocol names. + let mut protocols = Vec::new(); + let mut remaining: &[u8] = &msg; + loop { + // A well-formed message must be terminated with a newline. + if remaining == &[b'\n'] { + break + } else if protocols.len() == MAX_PROTOCOLS { + return Err(ProtocolError::TooManyProtocols) + } + + // Decode the length of the next protocol name and check that + // it ends with a line feed. + let (len, tail) = uvi::decode::usize(remaining)?; + if len == 0 || len > tail.len() || tail[len - 1] != b'\n' { return Err(ProtocolError::InvalidMessage) } - let p = Protocol::try_from(Bytes::copy_from_slice(&rem[.. len - 1]))?; + + // Parse the protocol name. + let p = Protocol::try_from(Bytes::copy_from_slice(&tail[.. len - 1]))?; protocols.push(p); - remaining = &rem[len ..] + + // Skip ahead to the next protocol. + remaining = &tail[len ..]; } return Ok(Message::Protocols(protocols)); From 1bd013c8432b88690b845594cf656a8e6756a28a Mon Sep 17 00:00:00 2001 From: Roman Borschel Date: Wed, 18 Nov 2020 15:52:33 +0100 Subject: [PATCH 02/19] [swarm] Configurable and "infinite" scores for external addresses. (#1842) * Add "infinite" scores for external addresses. Extend address scores with an infinite cardinal, permitting addresses to be retained "forever" or until explicitly removed. Expose (external) address scores on the API. * Update swarm/src/registry.rs Co-authored-by: Pierre Krieger * Fix compilation. * Update CHANGELOG Co-authored-by: Pierre Krieger --- misc/core-derive/src/lib.rs | 4 +- protocols/gossipsub/src/behaviour.rs | 4 +- protocols/identify/src/identify.rs | 8 +- protocols/kad/src/behaviour.rs | 2 +- protocols/request-response/src/throttled.rs | 4 +- swarm/CHANGELOG.md | 5 + swarm/src/behaviour.rs | 20 +- swarm/src/lib.rs | 57 +-- swarm/src/registry.rs | 362 +++++++++++++++----- 9 files changed, 346 insertions(+), 120 deletions(-) diff --git a/misc/core-derive/src/lib.rs b/misc/core-derive/src/lib.rs index 762a8bc74822..87a2a9637938 100644 --- a/misc/core-derive/src/lib.rs +++ b/misc/core-derive/src/lib.rs @@ -450,8 +450,8 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream { event: #wrapped_event, }); } - std::task::Poll::Ready(#network_behaviour_action::ReportObservedAddr { address }) => { - return std::task::Poll::Ready(#network_behaviour_action::ReportObservedAddr { address }); + std::task::Poll::Ready(#network_behaviour_action::ReportObservedAddr { address, score }) => { + return std::task::Poll::Ready(#network_behaviour_action::ReportObservedAddr { address, score }); } std::task::Poll::Pending => break, } diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 2761a877771d..fb17190507b1 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -1412,8 +1412,8 @@ impl NetworkBehaviour for Gossipsub { NetworkBehaviourAction::DialPeer { peer_id, condition } => { NetworkBehaviourAction::DialPeer { peer_id, condition } } - NetworkBehaviourAction::ReportObservedAddr { address } => { - NetworkBehaviourAction::ReportObservedAddr { address } + NetworkBehaviourAction::ReportObservedAddr { address, score } => { + NetworkBehaviourAction::ReportObservedAddr { address, score } } }); } diff --git a/protocols/identify/src/identify.rs b/protocols/identify/src/identify.rs index 6c6c0613bc30..667a3f224be2 100644 --- a/protocols/identify/src/identify.rs +++ b/protocols/identify/src/identify.rs @@ -30,6 +30,7 @@ use libp2p_core::{ upgrade::{ReadOneError, UpgradeError} }; use libp2p_swarm::{ + AddressScore, NegotiatedSubstream, NetworkBehaviour, NetworkBehaviourAction, @@ -47,6 +48,10 @@ use std::{ /// Network behaviour that automatically identifies nodes periodically, returns information /// about them, and answers identify queries from other nodes. +/// +/// All external addresses of the local node supposedly observed by remotes +/// are reported via [`NetworkBehaviourAction::ReportObservedAddr`] with a +/// [score](AddressScore) of `1`. pub struct Identify { /// Protocol version to send back to remotes. protocol_version: String, @@ -143,6 +148,7 @@ impl NetworkBehaviour for Identify { self.events.push_back( NetworkBehaviourAction::ReportObservedAddr { address: remote.observed_addr, + score: AddressScore::Finite(1), }); } IdentifyHandlerEvent::Identify(sender) => { @@ -187,7 +193,7 @@ impl NetworkBehaviour for Identify { .map(|p| String::from_utf8_lossy(&p).to_string()) .collect(); - let mut listen_addrs: Vec<_> = params.external_addresses().collect(); + let mut listen_addrs: Vec<_> = params.external_addresses().map(|r| r.addr).collect(); listen_addrs.extend(params.listened_addresses()); let mut sending = 0; diff --git a/protocols/kad/src/behaviour.rs b/protocols/kad/src/behaviour.rs index e55d1e4d6542..2034e66e6475 100644 --- a/protocols/kad/src/behaviour.rs +++ b/protocols/kad/src/behaviour.rs @@ -1046,7 +1046,7 @@ where phase: AddProviderPhase::GetClosestPeers } => { let provider_id = params.local_peer_id().clone(); - let external_addresses = params.external_addresses().collect(); + let external_addresses = params.external_addresses().map(|r| r.addr).collect(); let inner = QueryInner::new(QueryInfo::AddProvider { context, key, diff --git a/protocols/request-response/src/throttled.rs b/protocols/request-response/src/throttled.rs index 3b67328fa82c..d66b7efb46e5 100644 --- a/protocols/request-response/src/throttled.rs +++ b/protocols/request-response/src/throttled.rs @@ -553,8 +553,8 @@ where NetworkBehaviourAction::DialPeer { peer_id, condition }, | NetworkBehaviourAction::NotifyHandler { peer_id, handler, event } => NetworkBehaviourAction::NotifyHandler { peer_id, handler, event }, - | NetworkBehaviourAction::ReportObservedAddr { address } => - NetworkBehaviourAction::ReportObservedAddr { address } + | NetworkBehaviourAction::ReportObservedAddr { address, score } => + NetworkBehaviourAction::ReportObservedAddr { address, score } }; return Poll::Ready(event) diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md index 2c2b0d355044..734b39144819 100644 --- a/swarm/CHANGELOG.md +++ b/swarm/CHANGELOG.md @@ -2,6 +2,11 @@ - Update `libp2p-core`. +- Expose configurable scores for external addresses, as well as + the ability to remove them and to add addresses that are + retained "forever" (or until explicitly removed). + [PR 1842](https://github.com/libp2p/rust-libp2p/pull/1842). + # 0.24.0 [2020-11-09] - Update dependencies. diff --git a/swarm/src/behaviour.rs b/swarm/src/behaviour.rs index 8da3425e01d5..4ac77cb403c3 100644 --- a/swarm/src/behaviour.rs +++ b/swarm/src/behaviour.rs @@ -18,6 +18,7 @@ // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. +use crate::{AddressScore, AddressRecord}; use crate::protocols_handler::{IntoProtocolsHandler, ProtocolsHandler}; use libp2p_core::{ConnectedPoint, Multiaddr, PeerId, connection::{ConnectionId, ListenerId}}; use std::{error, task::Context, task::Poll}; @@ -182,7 +183,7 @@ pub trait PollParameters { /// Iterator returned by [`listened_addresses`](PollParameters::listened_addresses). type ListenedAddressesIter: ExactSizeIterator; /// Iterator returned by [`external_addresses`](PollParameters::external_addresses). - type ExternalAddressesIter: ExactSizeIterator; + type ExternalAddressesIter: ExactSizeIterator; /// Returns the list of protocol the behaviour supports when a remote negotiates a protocol on /// an inbound substream. @@ -269,8 +270,9 @@ pub enum NetworkBehaviourAction { event: TInEvent, }, - /// Informs the `Swarm` about a multi-address observed by a remote for - /// the local node. + /// Informs the `Swarm` about an address observed by a remote for + /// the local node by which the local node is supposedly publicly + /// reachable. /// /// It is advisable to issue `ReportObservedAddr` actions at a fixed frequency /// per node. This way address information will be more accurate over time @@ -278,6 +280,10 @@ pub enum NetworkBehaviourAction { ReportObservedAddr { /// The observed address of the local node. address: Multiaddr, + /// The score to associate with this observation, i.e. + /// an indicator for the trusworthiness of this address + /// relative to other observed addresses. + score: AddressScore, }, } @@ -297,8 +303,8 @@ impl NetworkBehaviourAction { handler, event: f(event) }, - NetworkBehaviourAction::ReportObservedAddr { address } => - NetworkBehaviourAction::ReportObservedAddr { address } + NetworkBehaviourAction::ReportObservedAddr { address, score } => + NetworkBehaviourAction::ReportObservedAddr { address, score } } } @@ -313,8 +319,8 @@ impl NetworkBehaviourAction { NetworkBehaviourAction::DialPeer { peer_id, condition }, NetworkBehaviourAction::NotifyHandler { peer_id, handler, event } => NetworkBehaviourAction::NotifyHandler { peer_id, handler, event }, - NetworkBehaviourAction::ReportObservedAddr { address } => - NetworkBehaviourAction::ReportObservedAddr { address } + NetworkBehaviourAction::ReportObservedAddr { address, score } => + NetworkBehaviourAction::ReportObservedAddr { address, score } } } } diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 146ed3a48d8f..f9009675416b 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -82,6 +82,7 @@ pub use protocols_handler::{ OneShotHandlerConfig, SubstreamProtocol }; +pub use registry::{AddressScore, AddressRecord, AddAddressResult}; use protocols_handler::{ NodeHandlerWrapperBuilder, @@ -397,34 +398,44 @@ where TBehaviour: NetworkBehaviour, me.network.listen_addrs() } - /// Returns an iterator that produces the list of addresses that other nodes can use to reach - /// us. - pub fn external_addresses(me: &Self) -> impl Iterator { - me.external_addrs.iter() - } - /// Returns the peer ID of the swarm passed as parameter. pub fn local_peer_id(me: &Self) -> &PeerId { &me.network.local_peer_id() } - /// Adds an external address. + /// Returns an iterator for [`AddressRecord`]s of external addresses + /// of the local node, in decreasing order of their current + /// [score](AddressScore). + pub fn external_addresses(me: &Self) -> impl Iterator { + me.external_addrs.iter() + } + + /// Adds an external address record for the local node. /// - /// An external address is an address we are listening on but that accounts for things such as - /// NAT traversal. - pub fn add_external_address(me: &mut Self, addr: Multiaddr) { - me.external_addrs.add(addr) + /// An external address is an address of the local node known to + /// be (likely) reachable for other nodes, possibly taking into + /// account NAT. The external addresses of the local node may be + /// shared with other nodes by the `NetworkBehaviour`. + /// + /// The associated score determines both the position of the address + /// in the list of external addresses (which can determine the + /// order in which addresses are used to connect to) as well as + /// how long the address is retained in the list, depending on + /// how frequently it is reported by the `NetworkBehaviour` via + /// [`NetworkBehaviourAction::ReportObservedAddr`] or explicitly + /// through this method. + pub fn add_external_address(me: &mut Self, a: Multiaddr, s: AddressScore) -> AddAddressResult { + me.external_addrs.add(a, s) } - /// Returns the connection info for an arbitrary connection with the peer, or `None` - /// if there is no connection to that peer. - // TODO: should take &self instead of &mut self, but the API in network requires &mut - pub fn connection_info(me: &mut Self, peer_id: &PeerId) -> Option { - if let Some(mut n) = me.network.peer(peer_id.clone()).into_connected() { - Some(n.some_connection().info().clone()) - } else { - None - } + /// Removes an external address of the local node, regardless of + /// its current score. See [`ExpandedSwarm::add_external_address`] + /// for details. + /// + /// Returns `true` if the address existed and was removed, `false` + /// otherwise. + pub fn remove_external_address(me: &mut Self, addr: &Multiaddr) -> bool { + me.external_addrs.remove(addr) } /// Bans a peer by its peer ID. @@ -732,12 +743,12 @@ where TBehaviour: NetworkBehaviour, } } }, - Poll::Ready(NetworkBehaviourAction::ReportObservedAddr { address }) => { + Poll::Ready(NetworkBehaviourAction::ReportObservedAddr { address, score }) => { for addr in this.network.address_translation(&address) { - if this.external_addrs.iter().all(|a| *a != addr) { + if this.external_addrs.iter().all(|a| &a.addr != &addr) { this.behaviour.inject_new_external_addr(&addr); } - this.external_addrs.add(addr); + this.external_addrs.add(addr, score); } }, } diff --git a/swarm/src/registry.rs b/swarm/src/registry.rs index ad00135d9ad5..935e72e04f99 100644 --- a/swarm/src/registry.rs +++ b/swarm/src/registry.rs @@ -20,29 +20,143 @@ use libp2p_core::Multiaddr; use smallvec::SmallVec; -use std::{collections::VecDeque, num::NonZeroUsize}; +use std::{collections::VecDeque, cmp::Ordering, num::NonZeroUsize}; +use std::ops::{Add, Sub}; -/// Hold a ranked collection of [`Multiaddr`] values. +/// A ranked collection of [`Multiaddr`] values. +/// +/// Every address has an associated [score](`AddressScore`) and iterating +/// over the addresses will return them in order from highest to lowest score. +/// +/// In addition to the currently held addresses and their score, the collection +/// keeps track of a limited history of the most-recently added addresses. +/// This history determines how address scores are reduced over time as old +/// scores expire in the context of new addresses being added: +/// +/// * An address's score is increased by a given amount whenever it is +/// [(re-)added](Addresses::add) to the collection. +/// * An address's score is decreased by the same amount used when it +/// was added when the least-recently seen addition is (as per the +/// limited history) for this address in the context of [`Addresses::add`]. +/// * If an address's score reaches 0 in the context of [`Addresses::add`], +/// it is removed from the collection. /// -/// Every address has an associated score and iterating over addresses will return them -/// in order from highest to lowest. When reaching the limit, addresses with the lowest -/// score will be dropped first. #[derive(Debug, Clone)] pub struct Addresses { - /// The ranked sequence of addresses. - registry: SmallVec<[Record; 8]>, - /// Number of historical reports. Similar to `reports.capacity()`. + /// The ranked sequence of addresses, from highest to lowest score. + /// + /// By design, the number of finitely scored addresses stored here is + /// never larger (but may be smaller) than the number of historic `reports` + /// at any time. + registry: SmallVec<[AddressRecord; 8]>, + /// The configured limit of the `reports` history of added addresses, + /// and thus also of the size of the `registry` w.r.t. finitely scored + /// addresses. limit: NonZeroUsize, - /// Queue of last reports. Every new report is added to the queue. If the queue reaches its - /// capacity, we also pop the first element. - reports: VecDeque, + /// The limited history of added addresses. If the queue reaches the `limit`, + /// the first record, i.e. the least-recently added, is removed in the + /// context of [`Addresses::add`] and the corresponding record in the + /// `registry` has its score reduced accordingly. + reports: VecDeque, } -// An address record associates a score to a Multiaddr. +/// An record in a prioritised list of addresses. #[derive(Clone, Debug, PartialEq, Eq)] -struct Record { +#[non_exhaustive] +pub struct AddressRecord { + pub addr: Multiaddr, + pub score: AddressScore, +} + +/// A report tracked for a finitely scored address. +#[derive(Debug, Clone)] +struct Report { + addr: Multiaddr, score: u32, - addr: Multiaddr +} + +impl AddressRecord { + fn new(addr: Multiaddr, score: AddressScore) -> Self { + AddressRecord { + addr, score, + } + } +} + +/// The "score" of an address w.r.t. an ordered collection of addresses. +/// +/// A score is a measure of the trusworthyness of a particular +/// observation of an address. The same address may be repeatedly +/// reported with the same or differing scores. +#[derive(PartialEq, Eq, Debug, Clone, Copy, Hash)] +pub enum AddressScore { + /// The score is "infinite", i.e. an address with this score is never + /// purged from the associated address records and remains sorted at + /// the beginning (possibly with other `Infinite`ly scored addresses). + Infinite, + /// The score is finite, i.e. an address with this score has + /// its score increased and decreased as per the frequency of + /// reports (i.e. additions) of the same address relative to + /// the reports of other addresses. + Finite(u32), +} + +impl AddressScore { + fn is_zero(&self) -> bool { + &AddressScore::Finite(0) == self + } +} + +impl PartialOrd for AddressScore { + fn partial_cmp(&self, other: &AddressScore) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for AddressScore { + fn cmp(&self, other: &AddressScore) -> Ordering { + // Semantics of cardinal numbers with a single infinite cardinal. + match (self, other) { + (AddressScore::Infinite, AddressScore::Infinite) => + Ordering::Equal, + (AddressScore::Infinite, AddressScore::Finite(_)) => + Ordering::Greater, + (AddressScore::Finite(_), AddressScore::Infinite) => + Ordering::Less, + (AddressScore::Finite(a), AddressScore::Finite(b)) => + a.cmp(b), + } + } +} + +impl Add for AddressScore { + type Output = AddressScore; + + fn add(self, rhs: AddressScore) -> Self::Output { + // Semantics of cardinal numbers with a single infinite cardinal. + match (self, rhs) { + (AddressScore::Infinite, AddressScore::Infinite) => + AddressScore::Infinite, + (AddressScore::Infinite, AddressScore::Finite(_)) => + AddressScore::Infinite, + (AddressScore::Finite(_), AddressScore::Infinite) => + AddressScore::Infinite, + (AddressScore::Finite(a), AddressScore::Finite(b)) => + AddressScore::Finite(a.saturating_add(b)) + } + } +} + +impl Sub for AddressScore { + type Output = AddressScore; + + fn sub(self, rhs: u32) -> Self::Output { + // Semantics of cardinal numbers with a single infinite cardinal. + match self { + AddressScore::Infinite => AddressScore::Infinite, + AddressScore::Finite(score) => AddressScore::Finite(score.saturating_sub(rhs)) + } + } } impl Default for Addresses { @@ -51,8 +165,16 @@ impl Default for Addresses { } } +/// The result of adding an address to an ordered list of +/// addresses with associated scores. +pub enum AddAddressResult { + Inserted, + Updated, +} + impl Addresses { - /// Create a new address collection of bounded length. + /// Create a new ranked address collection with the given size limit + /// for [finitely scored](AddressScore::Finite) addresses. pub fn new(limit: NonZeroUsize) -> Self { Addresses { registry: SmallVec::new(), @@ -63,40 +185,61 @@ impl Addresses { /// Add a [`Multiaddr`] to the collection. /// - /// Adding an existing address is interpreted as additional - /// confirmation and thus increases its score. - pub fn add(&mut self, a: Multiaddr) { - - let oldest = if self.reports.len() == self.limit.get() { - self.reports.pop_front() - } else { - None - }; - - if let Some(oldest) = oldest { - if let Some(in_registry) = self.registry.iter_mut().find(|r| r.addr == oldest) { - in_registry.score = in_registry.score.saturating_sub(1); + /// If the given address already exists in the collection, + /// the given score is added to the current score of the address. + /// + /// If the collection has already observed the configured + /// number of address additions, the least-recently added address + /// as per this limited history has its score reduced by the amount + /// used in this prior report, with removal from the collection + /// occurring when the score drops to 0. + pub fn add(&mut self, addr: Multiaddr, score: AddressScore) -> AddAddressResult { + // If enough reports (i.e. address additions) occurred, reduce + // the score of the least-recently added address. + if self.reports.len() == self.limit.get() { + let old_report = self.reports.pop_front().expect("len = limit > 0"); + // If the address is still in the collection, decrease its score. + if let Some(record) = self.registry.iter_mut().find(|r| r.addr == old_report.addr) { + record.score = record.score - old_report.score; isort(&mut self.registry); } } // Remove addresses that have a score of 0. - while self.registry.last().map(|e| e.score == 0).unwrap_or(false) { + while self.registry.last().map(|e| e.score.is_zero()).unwrap_or(false) { self.registry.pop(); } - self.reports.push_back(a.clone()); + // If the address score is finite, remember this report. + if let AddressScore::Finite(score) = score { + self.reports.push_back(Report { addr: addr.clone(), score }); + } + // If the address is already in the collection, increase its score. for r in &mut self.registry { - if r.addr == a { - r.score = r.score.saturating_add(1); + if r.addr == addr { + r.score = r.score + score; isort(&mut self.registry); - return + return AddAddressResult::Updated } } - let r = Record { score: 1, addr: a }; - self.registry.push(r) + // It is a new record. + self.registry.push(AddressRecord::new(addr, score)); + AddAddressResult::Inserted + } + + /// Explicitly remove an address from the collection. + /// + /// Returns `true` if the address existed in the collection + /// and was thus removed, false otherwise. + pub fn remove(&mut self, addr: &Multiaddr) -> bool { + if let Some(pos) = self.registry.iter().position(|r| &r.addr == addr) { + self.registry.remove(pos); + true + } else { + false + } } /// Return an iterator over all [`Multiaddr`] values. @@ -117,12 +260,12 @@ impl Addresses { /// An iterator over [`Multiaddr`] values. #[derive(Clone)] pub struct AddressIter<'a> { - items: &'a [Record], + items: &'a [AddressRecord], offset: usize } impl<'a> Iterator for AddressIter<'a> { - type Item = &'a Multiaddr; + type Item = &'a AddressRecord; fn next(&mut self) -> Option { if self.offset == self.items.len() { @@ -130,7 +273,7 @@ impl<'a> Iterator for AddressIter<'a> { } let item = &self.items[self.offset]; self.offset += 1; - Some(&item.addr) + Some(&item) } fn size_hint(&self) -> (usize, Option) { @@ -144,15 +287,15 @@ impl<'a> ExactSizeIterator for AddressIter<'a> {} /// An iterator over [`Multiaddr`] values. #[derive(Clone)] pub struct AddressIntoIter { - items: SmallVec<[Record; 8]>, + items: SmallVec<[AddressRecord; 8]>, } impl Iterator for AddressIntoIter { - type Item = Multiaddr; + type Item = AddressRecord; fn next(&mut self) -> Option { if !self.items.is_empty() { - Some(self.items.remove(0).addr) + Some(self.items.remove(0)) } else { None } @@ -167,7 +310,7 @@ impl Iterator for AddressIntoIter { impl ExactSizeIterator for AddressIntoIter {} // Reverse insertion sort. -fn isort(xs: &mut [Record]) { +fn isort(xs: &mut [AddressRecord]) { for i in 1 .. xs.len() { for j in (1 ..= i).rev() { if xs[j].score <= xs[j - 1].score { @@ -181,16 +324,34 @@ fn isort(xs: &mut [Record]) { #[cfg(test)] mod tests { use libp2p_core::multiaddr::{Multiaddr, Protocol}; - use quickcheck::{Arbitrary, Gen, QuickCheck}; + use quickcheck::*; use rand::Rng; - use std::num::NonZeroUsize; - use super::{isort, Addresses, Record}; + use std::num::{NonZeroUsize, NonZeroU8}; + use super::*; + + impl Arbitrary for AddressScore { + fn arbitrary(g: &mut G) -> AddressScore { + if g.gen_range(0, 10) == 0 { // ~10% "Infinitely" scored addresses + AddressScore::Infinite + } else { + AddressScore::Finite(g.gen()) + } + } + } + + impl Arbitrary for AddressRecord { + fn arbitrary(g: &mut G) -> Self { + let addr = Protocol::Tcp(g.gen::() % 256).into(); + let score = AddressScore::arbitrary(g); + AddressRecord::new(addr, score) + } + } #[test] fn isort_sorts() { - fn property(xs: Vec) -> bool { + fn property(xs: Vec) { let mut xs = xs.into_iter() - .map(|s| Record { score: s, addr: Multiaddr::empty() }) + .map(|score| AddressRecord::new(Multiaddr::empty(), score)) .collect::>(); isort(&mut xs); @@ -198,61 +359,98 @@ mod tests { for i in 1 .. xs.len() { assert!(xs[i - 1].score >= xs[i].score) } - - true } - QuickCheck::new().quickcheck(property as fn(Vec) -> bool) + + quickcheck(property as fn(_)); } #[test] - fn old_reports_disappear() { - let mut addresses = Addresses::default(); - - // Add an address a single time. - let single: Multiaddr = "/tcp/2108".parse().unwrap(); - addresses.add(single.clone()); - assert!(addresses.iter().find(|a| **a == single).is_some()); - - // Then fill `addresses` with random stuff. - let other: Multiaddr = "/tcp/120".parse().unwrap(); - for _ in 0 .. 2000 { - addresses.add(other.clone()); + fn score_retention() { + fn prop(first: AddressRecord, other: AddressRecord) -> TestResult { + if first.addr == other.addr { + return TestResult::discard() + } + + let mut addresses = Addresses::default(); + + // Add the first address. + addresses.add(first.addr.clone(), first.score); + assert!(addresses.iter().any(|a| &a.addr == &first.addr)); + + // Add another address so often that the initial report of + // the first address may be purged and, since it was the + // only report, the address removed. + for _ in 0 .. addresses.limit.get() + 1 { + addresses.add(other.addr.clone(), other.score); + } + + let exists = addresses.iter().any(|a| &a.addr == &first.addr); + + match (first.score, other.score) { + // Only finite scores push out other finite scores. + (AddressScore::Finite(_), AddressScore::Finite(_)) => assert!(!exists), + _ => assert!(exists), + } + + TestResult::passed() } - // Check that `single` disappeared from the list. - assert!(addresses.iter().find(|a| **a == single).is_none()); + quickcheck(prop as fn(_,_) -> _); } #[test] - fn record_score_equals_last_n_reports() { - #[derive(PartialEq, Eq, Clone, Hash, Debug)] - struct Ma(Multiaddr); + fn finitely_scored_address_limit() { + fn prop(reports: Vec, limit: NonZeroU8) { + let mut addresses = Addresses::new(limit.into()); - impl Arbitrary for Ma { - fn arbitrary(g: &mut G) -> Self { - Ma(Protocol::Tcp(g.gen::() % 16).into()) + // Add all reports. + for r in reports { + addresses.add(r.addr, r.score); } + + // Count the finitely scored addresses. + let num_finite = addresses.iter().filter(|r| match r { + AddressRecord { score: AddressScore::Finite(_), .. } => true, + _ => false, + }).count(); + + // Check against the limit. + assert!(num_finite <= limit.get() as usize); } - fn property(xs: Vec, n: u8) -> bool { - let n = std::cmp::max(n, 1); - let mut addresses = Addresses::new(NonZeroUsize::new(usize::from(n)).unwrap()); - for Ma(a) in &xs { - addresses.add(a.clone()) + quickcheck(prop as fn(_,_)); + } + + #[test] + fn record_score_sum() { + fn prop(records: Vec) -> bool { + // Make sure the address collection can hold all reports. + let n = std::cmp::max(records.len(), 1); + let mut addresses = Addresses::new(NonZeroUsize::new(n).unwrap()); + + // Add all address reports to the collection. + for r in records.iter() { + addresses.add(r.addr.clone(), r.score.clone()); } + + // Check that each address in the registry has the expected score. for r in &addresses.registry { - let count = xs.iter() - .rev() - .take(usize::from(n)) - .filter(|Ma(x)| x == &r.addr) - .count(); - if r.score as usize != count { + let expected_score = records.iter().fold( + None::, |sum, rec| + if &rec.addr == &r.addr { + sum.map_or(Some(rec.score), |s| Some(s + rec.score)) + } else { + sum + }); + + if Some(r.score) != expected_score { return false } } + true } - QuickCheck::new().quickcheck(property as fn(Vec, u8) -> bool) + quickcheck(prop as fn(_) -> _) } } From cef75ab7e11fa37650c106d89b7ef5409d442b00 Mon Sep 17 00:00:00 2001 From: Roman Borschel Date: Mon, 23 Nov 2020 17:22:15 +0100 Subject: [PATCH 03/19] [core/swarm] Refactor and extend configurable connection limits. (#1848) * Refactor and extend configurable connection limits. To better track different connection counts, permit configurable limits for these counts and make these available for inspection efficiently, introduce dedicated connection counters via a `ConnectionCounters` structure that is exposed on the API via the `NetworkInfo`. All connection or connection states that are counted in this way can also have effective configurable limits. * Cleanup * Add missing file. * Refine naming and config API. * Update core/CHANGELOG.md Co-authored-by: Max Inden * Update core/CHANGELOG.md Co-authored-by: Max Inden Co-authored-by: Max Inden --- core/CHANGELOG.md | 9 + core/src/connection.rs | 5 +- core/src/connection/pool.rs | 317 ++++++++++++++++++++++--------- core/src/network.rs | 105 +++++----- core/src/network/peer.rs | 17 +- core/tests/connection_limits.rs | 162 ++++++++++++++++ core/tests/network_dial_error.rs | 84 +------- core/tests/util.rs | 28 ++- swarm/CHANGELOG.md | 3 + swarm/src/lib.rs | 41 ++-- 10 files changed, 501 insertions(+), 270 deletions(-) create mode 100644 core/tests/connection_limits.rs diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index 724c55a0cda4..071218bc5715 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -1,5 +1,14 @@ # 0.25.0 [unreleased] +- The `NetworkConfig` API is now a builder that moves `self`. + [PR 1848](https://github.com/libp2p/rust-libp2p/pull/1848/). + +- New configurable connection limits for established connections and + dedicated connection counters. Removed the connection limit dedicated + to outgoing pending connection _per peer_. Connection limits are now + represented by `u32` intead of `usize` types. + [PR 1848](https://github.com/libp2p/rust-libp2p/pull/1848/). + - Update `multihash`. - Update `multistream-select`. diff --git a/core/src/connection.rs b/core/src/connection.rs index 9908c64589d0..f64801b944c9 100644 --- a/core/src/connection.rs +++ b/core/src/connection.rs @@ -32,6 +32,7 @@ pub use listeners::{ListenerId, ListenersStream, ListenersEvent}; pub use manager::ConnectionId; pub use substream::{Substream, SubstreamEndpoint, Close}; pub use pool::{EstablishedConnection, EstablishedConnectionIter, PendingConnection}; +pub use pool::{ConnectionLimits, ConnectionCounters}; use crate::muxing::StreamMuxer; use crate::{Multiaddr, PeerId}; @@ -326,9 +327,9 @@ impl<'a> OutgoingInfo<'a> { #[derive(Debug, Clone)] pub struct ConnectionLimit { /// The maximum number of connections. - pub limit: usize, + pub limit: u32, /// The current number of connections. - pub current: usize, + pub current: u32, } impl fmt::Display for ConnectionLimit { diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index 6c47704b28e3..e40c937cbf66 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -48,8 +48,8 @@ use std::{convert::TryFrom as _, error, fmt, num::NonZeroU32, task::Context, tas pub struct Pool { local_id: PeerId, - /// The configuration of the pool. - limits: PoolLimits, + /// The connection counter(s). + counters: ConnectionCounters, /// The connection manager that handles the connection I/O for both /// established and pending connections. @@ -75,9 +75,8 @@ impl fmt::Debug for Pool { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { - // TODO: More useful debug impl? f.debug_struct("Pool") - .field("limits", &self.limits) + .field("counters", &self.counters) .finish() } } @@ -183,13 +182,13 @@ where }, PoolEvent::ConnectionEvent { ref connection, ref event } => { f.debug_struct("PoolEvent::ConnectionEvent") - .field("conn_info", connection.info()) + .field("peer", connection.peer_id()) .field("event", event) .finish() }, PoolEvent::AddressChange { ref connection, ref new_endpoint, ref old_endpoint } => { f.debug_struct("PoolEvent::AddressChange") - .field("conn_info", connection.info()) + .field("peer", connection.peer_id()) .field("new_endpoint", new_endpoint) .field("old_endpoint", old_endpoint) .finish() @@ -205,11 +204,11 @@ impl pub fn new( local_id: PeerId, manager_config: ManagerConfig, - limits: PoolLimits + limits: ConnectionLimits ) -> Self { Pool { local_id, - limits, + counters: ConnectionCounters::new(limits), manager: Manager::new(manager_config), established: Default::default(), pending: Default::default(), @@ -217,9 +216,9 @@ impl } } - /// Gets the configured connection limits of the pool. - pub fn limits(&self) -> &PoolLimits { - &self.limits + /// Gets the dedicated connection counters. + pub fn counters(&self) -> &ConnectionCounters { + &self.counters } /// Adds a pending incoming connection to the pool in the form of a @@ -252,8 +251,8 @@ impl TMuxer: StreamMuxer + Send + Sync + 'static, TMuxer::OutboundSubstream: Send + 'static, { + self.counters.check_max_pending_incoming()?; let endpoint = info.to_connected_point(); - self.limits.check_incoming(|| self.iter_pending_incoming().count())?; Ok(self.add_pending(future, handler, endpoint, None)) } @@ -287,12 +286,7 @@ impl TMuxer: StreamMuxer + Send + Sync + 'static, TMuxer::OutboundSubstream: Send + 'static, { - self.limits.check_outgoing(|| self.iter_pending_outgoing().count())?; - - if let Some(peer) = &info.peer_id { - self.limits.check_outgoing_per_peer(|| self.num_peer_outgoing(peer))?; - } - + self.counters.check_max_pending_outgoing()?; let endpoint = info.to_connected_point(); Ok(self.add_pending(future, handler, endpoint, info.peer_id.cloned())) } @@ -350,6 +344,7 @@ impl }); let id = self.manager.add_pending(future, handler); + self.counters.inc_pending(&endpoint); self.pending.insert(id, (endpoint, peer)); id } @@ -377,13 +372,10 @@ impl TMuxer: StreamMuxer + Send + Sync + 'static, TMuxer::OutboundSubstream: Send + 'static, { - if let Some(limit) = self.limits.max_established_per_peer { - let current = self.num_peer_established(&i.peer_id); - if limit >= current { - return Err(ConnectionLimit { limit, current }) - } - } + self.counters.check_max_established(&i.endpoint)?; + self.counters.check_max_established_per_peer(self.num_peer_established(&i.peer_id))?; let id = self.manager.add(c, i.clone()); + self.counters.inc_established(&i.endpoint); self.established.entry(i.peer_id.clone()).or_default().insert(id, i.endpoint); Ok(id) } @@ -403,6 +395,7 @@ impl Some(PoolConnection::Pending(PendingConnection { entry, pending: &mut self.pending, + counters: &mut self.counters, })), None => None } @@ -429,6 +422,7 @@ impl Some(PendingConnection { entry, pending: &mut self.pending, + counters: &mut self.counters, }), _ => unreachable!("by consistency of `self.pending` with `self.manager`") } @@ -445,7 +439,7 @@ impl /// Returns the number of connected peers, i.e. those with at least one /// established connection in the pool. - pub fn num_connected(&self) -> usize { + pub fn num_peers(&self) -> usize { self.established.len() } @@ -462,7 +456,7 @@ impl if let Some(conns) = self.established.get(peer) { // Count upwards because we push to / pop from the end. See also `Pool::poll`. let mut num_established = 0; - for &id in conns.keys() { + for (&id, endpoint) in conns.iter() { match self.manager.entry(id) { Some(manager::Entry::Established(e)) => { let connected = e.remove(); @@ -473,6 +467,7 @@ impl }, _ => {} } + self.counters.dec_established(endpoint); } } self.established.remove(peer); @@ -490,30 +485,15 @@ impl } } for id in aborted { - self.pending.remove(&id); + if let Some((endpoint, _)) = self.pending.remove(&id) { + self.counters.dec_pending(&endpoint); + } } } - /// Counts the number of established connections in the pool. - pub fn num_established(&self) -> usize { - self.established.iter().fold(0, |n, (_, conns)| n + conns.len()) - } - - /// Counts the number of pending connections in the pool. - pub fn num_pending(&self) -> usize { - self.iter_pending_info().count() - } - /// Counts the number of established connections to the given peer. - pub fn num_peer_established(&self, peer: &PeerId) -> usize { - self.established.get(peer).map_or(0, |conns| conns.len()) - } - - /// Counts the number of pending outgoing connections to the given peer. - pub fn num_peer_outgoing(&self, peer: &PeerId) -> usize { - self.iter_pending_outgoing() - .filter(|info| info.peer_id == Some(peer)) - .count() + pub fn num_peer_established(&self, peer: &PeerId) -> u32 { + num_peer_established(&self.established, peer) } /// Returns an iterator over all established connections of `peer`. @@ -620,6 +600,7 @@ impl match item { manager::Event::PendingConnectionError { id, error, handler } => { if let Some((endpoint, peer)) = self.pending.remove(&id) { + self.counters.dec_pending(&endpoint); return Poll::Ready(PoolEvent::PendingConnectionError { id, endpoint, @@ -633,7 +614,9 @@ impl manager::Event::ConnectionClosed { id, connected, error } => { let num_established = if let Some(conns) = self.established.get_mut(&connected.peer_id) { - conns.remove(&id); + if let Some(endpoint) = conns.remove(&id) { + self.counters.dec_established(&endpoint); + } u32::try_from(conns.len()).unwrap() } else { 0 @@ -648,11 +631,24 @@ impl manager::Event::ConnectionEstablished { entry } => { let id = entry.id(); if let Some((endpoint, peer)) = self.pending.remove(&id) { - // Check connection limit. - let established = &self.established; - let current = || established.get(&entry.connected().peer_id) - .map_or(0, |conns| conns.len()); - if let Err(e) = self.limits.check_established(current) { + self.counters.dec_pending(&endpoint); + + // Check general established connection limit. + if let Err(e) = self.counters.check_max_established(&endpoint) { + let connected = entry.remove(); + return Poll::Ready(PoolEvent::PendingConnectionError { + id, + endpoint: connected.endpoint, + error: PendingConnectionError::ConnectionLimit(e), + handler: None, + peer, + pool: self + }) + } + + // Check per-peer established connection limit. + let current = num_peer_established(&self.established, &entry.connected().peer_id); + if let Err(e) = self.counters.check_max_established_per_peer(current) { let connected = entry.remove(); return Poll::Ready(PoolEvent::PendingConnectionError { id, @@ -663,6 +659,7 @@ impl pool: self }) } + // Peer ID checks must already have happened. See `add_pending`. if cfg!(debug_assertions) { if self.local_id == entry.connected().peer_id { @@ -674,11 +671,13 @@ impl } } } + // Add the connection to the pool. let peer = entry.connected().peer_id.clone(); let conns = self.established.entry(peer).or_default(); let num_established = NonZeroU32::new(u32::try_from(conns.len() + 1).unwrap()) .expect("n + 1 is always non-zero; qed"); + self.counters.inc_established(&endpoint); conns.insert(id, endpoint); match self.get(id) { Some(PoolConnection::Established(connection)) => @@ -736,6 +735,7 @@ pub enum PoolConnection<'a, TInEvent> { pub struct PendingConnection<'a, TInEvent> { entry: manager::PendingEntry<'a, TInEvent>, pending: &'a mut FnvHashMap)>, + counters: &'a mut ConnectionCounters, } impl @@ -758,7 +758,8 @@ impl /// Aborts the connection attempt, closing the connection. pub fn abort(self) { - self.pending.remove(&self.entry.id()); + let endpoint = self.pending.remove(&self.entry.id()).expect("`entry` is a pending entry").0; + self.counters.dec_pending(&endpoint); self.entry.abort(); } } @@ -790,24 +791,16 @@ impl EstablishedConnection<'_, TInEvent> { &self.entry.connected().endpoint } - /// Returns connection information obtained from the transport. - pub fn info(&self) -> &PeerId { + /// Returns the identity of the connected peer. + pub fn peer_id(&self) -> &PeerId { &self.entry.connected().peer_id } -} -impl<'a, TInEvent> EstablishedConnection<'a, TInEvent> -{ /// Returns the local connection ID. pub fn id(&self) -> ConnectionId { self.entry.id() } - /// Returns the identity of the connected peer. - pub fn peer_id(&self) -> &PeerId { - self.info() - } - /// (Asynchronously) sends an event to the connection handler. /// /// If the handler is not ready to receive the event, either because @@ -894,62 +887,196 @@ where } } -/// The configurable limits of a connection [`Pool`]. -#[derive(Debug, Clone, Default)] -pub struct PoolLimits { - pub max_outgoing: Option, - pub max_incoming: Option, - pub max_established_per_peer: Option, - pub max_outgoing_per_peer: Option, +/// Network connection information. +#[derive(Debug, Clone)] +pub struct ConnectionCounters { + /// The effective connection limits. + limits: ConnectionLimits, + /// The current number of incoming connections. + pending_incoming: u32, + /// The current number of outgoing connections. + pending_outgoing: u32, + /// The current number of established inbound connections. + established_incoming: u32, + /// The current number of established outbound connections. + established_outgoing: u32, } -impl PoolLimits { - fn check_established(&self, current: F) -> Result<(), ConnectionLimit> - where - F: FnOnce() -> usize - { - Self::check(current, self.max_established_per_peer) +impl ConnectionCounters { + fn new(limits: ConnectionLimits) -> Self { + Self { + limits, + pending_incoming: 0, + pending_outgoing: 0, + established_incoming: 0, + established_outgoing: 0, + } } - fn check_outgoing(&self, current: F) -> Result<(), ConnectionLimit> - where - F: FnOnce() -> usize - { - Self::check(current, self.max_outgoing) + /// The effective connection limits. + pub fn limits(&self) -> &ConnectionLimits { + &self.limits } - fn check_incoming(&self, current: F) -> Result<(), ConnectionLimit> - where - F: FnOnce() -> usize - { - Self::check(current, self.max_incoming) + /// The total number of connections, both pending and established. + pub fn num_connections(&self) -> u32 { + self.num_pending() + self.num_established() } - fn check_outgoing_per_peer(&self, current: F) -> Result<(), ConnectionLimit> - where - F: FnOnce() -> usize - { - Self::check(current, self.max_outgoing_per_peer) + /// The total number of pending connections, both incoming and outgoing. + pub fn num_pending(&self) -> u32 { + self.pending_incoming + self.pending_outgoing } - fn check(current: F, limit: Option) -> Result<(), ConnectionLimit> - where - F: FnOnce() -> usize + /// The number of incoming connections being established. + pub fn num_pending_incoming(&self) -> u32 { + self.pending_incoming + } + + /// The number of outgoing connections being established. + pub fn num_pending_outgoing(&self) -> u32 { + self.pending_outgoing + } + + /// The number of established incoming connections. + pub fn num_established_incoming(&self) -> u32 { + self.established_incoming + } + + /// The number of established outgoing connections. + pub fn num_established_outgoing(&self) -> u32 { + self.established_outgoing + } + + /// The total number of established connections. + pub fn num_established(&self) -> u32 { + self.established_outgoing + self.established_incoming + } + + fn inc_pending(&mut self, endpoint: &ConnectedPoint) { + match endpoint { + ConnectedPoint::Dialer { .. } => { self.pending_outgoing += 1; } + ConnectedPoint::Listener { .. } => { self.pending_incoming += 1; } + } + } + + fn dec_pending(&mut self, endpoint: &ConnectedPoint) { + match endpoint { + ConnectedPoint::Dialer { .. } => { self.pending_outgoing -= 1; } + ConnectedPoint::Listener { .. } => { self.pending_incoming -= 1; } + } + } + + fn inc_established(&mut self, endpoint: &ConnectedPoint) { + match endpoint { + ConnectedPoint::Dialer { .. } => { self.established_outgoing += 1; } + ConnectedPoint::Listener { .. } => { self.established_incoming += 1; } + } + } + + fn dec_established(&mut self, endpoint: &ConnectedPoint) { + match endpoint { + ConnectedPoint::Dialer { .. } => { self.established_outgoing -= 1; } + ConnectedPoint::Listener { .. } => { self.established_incoming -= 1; } + } + } + + fn check_max_pending_outgoing(&self) -> Result<(), ConnectionLimit> { + Self::check(self.pending_outgoing, self.limits.max_pending_outgoing) + } + + fn check_max_pending_incoming(&self) -> Result<(), ConnectionLimit> { + Self::check(self.pending_incoming, self.limits.max_pending_incoming) + } + + fn check_max_established(&self, endpoint: &ConnectedPoint) + -> Result<(), ConnectionLimit> { + match endpoint { + ConnectedPoint::Dialer { .. } => + Self::check(self.established_outgoing, self.limits.max_established_outgoing), + ConnectedPoint::Listener { .. } => { + Self::check(self.established_incoming, self.limits.max_established_incoming) + } + } + } + + fn check_max_established_per_peer(&self, current: u32) -> Result<(), ConnectionLimit> { + Self::check(current, self.limits.max_established_per_peer) + } + + fn check(current: u32, limit: Option) -> Result<(), ConnectionLimit> { if let Some(limit) = limit { - let current = current(); if current >= limit { return Err(ConnectionLimit { limit, current }) } } Ok(()) } + +} + +/// Counts the number of established connections to the given peer. +fn num_peer_established( + established: &FnvHashMap>, + peer: &PeerId +) -> u32 { + established.get(peer).map_or(0, |conns| + u32::try_from(conns.len()) + .expect("Unexpectedly large number of connections for a peer.")) +} + +/// The configurable connection limits. +/// +/// By default no connection limits apply. +#[derive(Debug, Clone, Default)] +pub struct ConnectionLimits { + max_pending_incoming: Option, + max_pending_outgoing: Option, + max_established_incoming: Option, + max_established_outgoing: Option, + max_established_per_peer: Option, +} + +impl ConnectionLimits { + /// Configures the maximum number of concurrently incoming connections being established. + pub fn with_max_pending_incoming(mut self, limit: Option) -> Self { + self.max_pending_incoming = limit; + self + } + + /// Configures the maximum number of concurrently outgoing connections being established. + pub fn with_max_pending_outgoing(mut self, limit: Option) -> Self { + self.max_pending_outgoing = limit; + self + } + + /// Configures the maximum number of concurrent established inbound connections. + pub fn with_max_established_incoming(mut self, limit: Option) -> Self { + self.max_established_incoming = limit; + self + } + + /// Configures the maximum number of concurrent established outbound connections. + pub fn with_max_established_outgoing(mut self, limit: Option) -> Self { + self.max_established_outgoing = limit; + self + } + + /// Configures the maximum number of concurrent established connections per peer, + /// regardless of direction (incoming or outgoing). + pub fn with_max_established_per_peer(mut self, limit: Option) -> Self { + self.max_established_per_peer = limit; + self + } } /// Information about a former established connection to a peer /// that was dropped via [`Pool::disconnect`]. struct Disconnected { + /// The unique identifier of the dropped connection. id: ConnectionId, + /// Information about the dropped connection. connected: Connected, /// The remaining number of established connections /// to the same peer. diff --git a/core/src/network.rs b/core/src/network.rs index b06f61654e9d..59444662cf78 100644 --- a/core/src/network.rs +++ b/core/src/network.rs @@ -21,6 +21,7 @@ mod event; pub mod peer; +pub use crate::connection::{ConnectionLimits, ConnectionCounters}; pub use event::{NetworkEvent, IncomingConnection}; pub use peer::Peer; @@ -43,7 +44,7 @@ use crate::{ PendingConnectionError, Substream, manager::ManagerConfig, - pool::{Pool, PoolEvent, PoolLimits}, + pool::{Pool, PoolEvent}, }, muxing::StreamMuxer, transport::{Transport, TransportError}, @@ -134,9 +135,9 @@ where TTrans: Transport + Clone, TMuxer: StreamMuxer, THandler: IntoConnectionHandler + Send + 'static, - THandler::Handler: ConnectionHandler, InEvent = TInEvent, OutEvent = TOutEvent> + Send + 'static, - ::OutboundOpenInfo: Send + 'static, // TODO: shouldn't be necessary - ::Error: error::Error + Send + 'static, + THandler::Handler: ConnectionHandler, InEvent = TInEvent, OutEvent = TOutEvent> + Send, + ::OutboundOpenInfo: Send, + ::Error: error::Error + Send, { /// Creates a new node events stream. pub fn new( @@ -148,7 +149,7 @@ where Network { local_peer_id, listeners: ListenersStream::new(transport), - pool: Pool::new(pool_local_id, config.manager_config, config.pool_limits), + pool: Pool::new(pool_local_id, config.manager_config, config.limits), dialing: Default::default(), } } @@ -244,15 +245,11 @@ where /// Returns information about the state of the `Network`. pub fn info(&self) -> NetworkInfo { - let num_connections_established = self.pool.num_established(); - let num_connections_pending = self.pool.num_pending(); - let num_connections = num_connections_established + num_connections_pending; - let num_peers = self.pool.num_connected(); + let num_peers = self.pool.num_peers(); + let connection_counters = self.pool.counters().clone(); NetworkInfo { num_peers, - num_connections, - num_connections_established, - num_connections_pending, + connection_counters, } } @@ -301,22 +298,6 @@ where self.dialing.keys() } - /// Gets the configured limit on pending incoming connections, - /// i.e. concurrent incoming connection attempts. - pub fn incoming_limit(&self) -> Option { - self.pool.limits().max_incoming - } - - /// The total number of established connections in the `Network`. - pub fn num_connections_established(&self) -> usize { - self.pool.num_established() - } - - /// The total number of pending connections in the `Network`. - pub fn num_connections_pending(&self) -> usize { - self.pool.num_pending() - } - /// Obtains a view of a [`Peer`] with the given ID in the network. pub fn peer(&mut self, peer_id: PeerId) -> Peer<'_, TTrans, TInEvent, TOutEvent, THandler> @@ -615,13 +596,22 @@ where #[derive(Clone, Debug)] pub struct NetworkInfo { /// The total number of connected peers. - pub num_peers: usize, - /// The total number of connections, both established and pending. - pub num_connections: usize, - /// The total number of pending connections, both incoming and outgoing. - pub num_connections_pending: usize, - /// The total number of established connections. - pub num_connections_established: usize, + num_peers: usize, + /// Counters of ongoing network connections. + connection_counters: ConnectionCounters, +} + +impl NetworkInfo { + /// The number of connected peers, i.e. peers with whom at least + /// one established connection exists. + pub fn num_peers(&self) -> usize { + self.num_peers + } + + /// Gets counters for ongoing network connections. + pub fn connection_counters(&self) -> &ConnectionCounters { + &self.connection_counters + } } /// The (optional) configuration for a [`Network`]. @@ -635,17 +625,25 @@ pub struct NetworkConfig { /// one "free" slot per task. Thus the given total `notify_handler_buffer_size` /// exposed for configuration on the `Network` is reduced by one. manager_config: ManagerConfig, - pool_limits: PoolLimits, + /// The effective connection limits. + limits: ConnectionLimits, } impl NetworkConfig { - pub fn set_executor(&mut self, e: Box) -> &mut Self { + /// Configures the executor to use for spawning connection background tasks. + pub fn with_executor(mut self, e: Box) -> Self { self.manager_config.executor = Some(e); self } - pub fn executor(&self) -> Option<&Box> { - self.manager_config.executor.as_ref() + /// Configures the executor to use for spawning connection background tasks, + /// only if no executor has already been configured. + pub fn or_else_with_executor(mut self, f: F) -> Self + where + F: FnOnce() -> Option> + { + self.manager_config.executor = self.manager_config.executor.or_else(f); + self } /// Sets the maximum number of events sent to a connection's background task @@ -655,7 +653,7 @@ impl NetworkConfig { /// When the buffer for a particular connection is full, `notify_handler` will no /// longer be able to deliver events to the associated `ConnectionHandler`, /// thus exerting back-pressure on the connection and peer API. - pub fn set_notify_handler_buffer_size(&mut self, n: NonZeroUsize) -> &mut Self { + pub fn with_notify_handler_buffer_size(mut self, n: NonZeroUsize) -> Self { self.manager_config.task_command_buffer_size = n.get() - 1; self } @@ -666,28 +664,14 @@ impl NetworkConfig { /// When the buffer is full, the background tasks of all connections will stall. /// In this way, the consumers of network events exert back-pressure on /// the network connection I/O. - pub fn set_connection_event_buffer_size(&mut self, n: usize) -> &mut Self { + pub fn with_connection_event_buffer_size(mut self, n: usize) -> Self { self.manager_config.task_event_buffer_size = n; self } - pub fn set_incoming_limit(&mut self, n: usize) -> &mut Self { - self.pool_limits.max_incoming = Some(n); - self - } - - pub fn set_outgoing_limit(&mut self, n: usize) -> &mut Self { - self.pool_limits.max_outgoing = Some(n); - self - } - - pub fn set_established_per_peer_limit(&mut self, n: usize) -> &mut Self { - self.pool_limits.max_established_per_peer = Some(n); - self - } - - pub fn set_outgoing_per_peer_limit(&mut self, n: usize) -> &mut Self { - self.pool_limits.max_outgoing_per_peer = Some(n); + /// Sets the connection limits to enforce. + pub fn with_connection_limits(mut self, limits: ConnectionLimits) -> Self { + self.limits = limits; self } } @@ -705,10 +689,9 @@ mod tests { #[test] fn set_executor() { NetworkConfig::default() - .set_executor(Box::new(Dummy)) - .set_executor(Box::new(|f| { + .with_executor(Box::new(Dummy)) + .with_executor(Box::new(|f| { async_std::task::spawn(f); })); } - } diff --git a/core/src/network/peer.rs b/core/src/network/peer.rs index 43aa55b71836..1f0d58ad4f0d 100644 --- a/core/src/network/peer.rs +++ b/core/src/network/peer.rs @@ -310,7 +310,7 @@ where } /// The number of established connections to the peer. - pub fn num_connections(&self) -> usize { + pub fn num_connections(&self) -> u32 { self.network.pool.num_peer_established(&self.peer_id) } @@ -448,12 +448,6 @@ where None } - /// The number of ongoing dialing attempts, i.e. pending outgoing connections - /// to this peer. - pub fn num_attempts(&self) -> usize { - self.network.pool.num_peer_outgoing(&self.peer_id) - } - /// Gets an iterator over all dialing (i.e. pending outgoing) connections to the peer. pub fn attempts<'b>(&'b mut self) -> DialingAttemptIter<'b, @@ -672,6 +666,15 @@ impl<'a, TInEvent, TOutEvent, THandler, TTransErr, THandlerErr> /// Obtains the next dialing connection, if any. pub fn next<'b>(&'b mut self) -> Option> { + // If the number of elements reduced, the current `DialingAttempt` has been + // aborted and iteration needs to continue from the previous position to + // account for the removed element. + let end = self.dialing.get(self.peer_id).map_or(0, |conns| conns.len()); + if self.end > end { + self.end = end; + self.pos -= 1; + } + if self.pos == self.end { return None } diff --git a/core/tests/connection_limits.rs b/core/tests/connection_limits.rs new file mode 100644 index 000000000000..4c9c31930660 --- /dev/null +++ b/core/tests/connection_limits.rs @@ -0,0 +1,162 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the "Software"), +// to deal in the Software without restriction, including without limitation +// the rights to use, copy, modify, merge, publish, distribute, sublicense, +// and/or sell copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +mod util; + +use futures::{ready, future::poll_fn}; +use libp2p_core::multiaddr::{multiaddr, Multiaddr}; +use libp2p_core::{ + PeerId, + connection::PendingConnectionError, + network::{NetworkEvent, NetworkConfig, ConnectionLimits}, +}; +use rand::Rng; +use std::task::Poll; +use util::{TestHandler, test_network}; + +#[test] +fn max_outgoing() { + let outgoing_limit = rand::thread_rng().gen_range(1, 10); + + let limits = ConnectionLimits::default().with_max_pending_outgoing(Some(outgoing_limit)); + let cfg = NetworkConfig::default().with_connection_limits(limits); + let mut network = test_network(cfg); + + let target = PeerId::random(); + for _ in 0 .. outgoing_limit { + network.peer(target.clone()) + .dial(Multiaddr::empty(), Vec::new(), TestHandler()) + .ok() + .expect("Unexpected connection limit."); + } + + let err = network.peer(target.clone()) + .dial(Multiaddr::empty(), Vec::new(), TestHandler()) + .expect_err("Unexpected dialing success."); + + assert_eq!(err.current, outgoing_limit); + assert_eq!(err.limit, outgoing_limit); + + let info = network.info(); + assert_eq!(info.num_peers(), 0); + assert_eq!(info.connection_counters().num_pending_outgoing(), outgoing_limit); + + // Abort all dialing attempts. + let mut peer = network.peer(target.clone()) + .into_dialing() + .expect("Unexpected peer state"); + + let mut attempts = peer.attempts(); + while let Some(attempt) = attempts.next() { + attempt.abort(); + } + + assert_eq!(network.info().connection_counters().num_pending_outgoing(), 0); +} + +#[test] +fn max_established_incoming() { + let limit = rand::thread_rng().gen_range(1, 10); + + fn config(limit: u32) -> NetworkConfig { + let limits = ConnectionLimits::default().with_max_established_incoming(Some(limit)); + NetworkConfig::default().with_connection_limits(limits) + } + + let mut network1 = test_network(config(limit)); + let mut network2 = test_network(config(limit)); + + let listen_addr = multiaddr![Ip4(std::net::Ipv4Addr::new(127,0,0,1)), Tcp(0u16)]; + let _ = network1.listen_on(listen_addr.clone()).unwrap(); + let (addr_sender, addr_receiver) = futures::channel::oneshot::channel(); + let mut addr_sender = Some(addr_sender); + + // Spawn the listener. + let listener = async_std::task::spawn(poll_fn(move |cx| { + loop { + match ready!(network1.poll(cx)) { + NetworkEvent::NewListenerAddress { listen_addr, .. } => { + addr_sender.take().unwrap().send(listen_addr).unwrap(); + } + NetworkEvent::IncomingConnection { connection, .. } => { + network1.accept(connection, TestHandler()).unwrap(); + } + NetworkEvent::ConnectionEstablished { .. } => {} + NetworkEvent::IncomingConnectionError { + error: PendingConnectionError::ConnectionLimit(err), .. + } => { + assert_eq!(err.limit, limit); + assert_eq!(err.limit, err.current); + let info = network1.info(); + let counters = info.connection_counters(); + assert_eq!(counters.num_established_incoming(), limit); + assert_eq!(counters.num_established(), limit); + return Poll::Ready(()) + } + e => panic!("Unexpected network event: {:?}", e) + } + } + })); + + // Spawn and block on the dialer. + async_std::task::block_on(async move { + let addr = addr_receiver.await.unwrap(); + let mut n = 0; + let _ = network2.dial(&addr, TestHandler()).unwrap(); + let mut expected_closed = None; + poll_fn(|cx| { + loop { + match ready!(network2.poll(cx)) { + NetworkEvent::ConnectionEstablished { connection, .. } => { + n += 1; + if n <= limit { + // Dial again until the limit is exceeded. + let id = network2.dial(&addr, TestHandler()).unwrap(); + if n == limit { + // The the next dialing attempt exceeds the limit, this + // is the connection we expected to get closed. + expected_closed = Some(id); + } + } else { + // This connection exceeds the limit for the listener and + // is expected to close shortly. For the dialer, these connections + // will first appear established before the listener closes them as + // a result of the limit violation. + assert_eq!(Some(connection.id()), expected_closed); + } + } + NetworkEvent::ConnectionClosed { id, .. } => { + assert_eq!(Some(id), expected_closed); + let info = network2.info(); + let counters = info.connection_counters(); + assert_eq!(counters.num_established_outgoing(), limit); + assert_eq!(counters.num_established(), limit); + return Poll::Ready(()) + } + e => panic!("Unexpected network event: {:?}", e) + } + } + }).await + }); + + // Wait for the listener to complete. + async_std::task::block_on(listener); +} + diff --git a/core/tests/network_dial_error.rs b/core/tests/network_dial_error.rs index d679775e0586..13e532d7016d 100644 --- a/core/tests/network_dial_error.rs +++ b/core/tests/network_dial_error.rs @@ -21,51 +21,22 @@ mod util; use futures::prelude::*; -use libp2p_core::identity; -use libp2p_core::multiaddr::{multiaddr, Multiaddr}; +use libp2p_core::multiaddr::multiaddr; use libp2p_core::{ - Network, PeerId, - Transport, connection::PendingConnectionError, - muxing::StreamMuxerBox, network::{NetworkEvent, NetworkConfig}, - transport, - upgrade, }; -use libp2p_noise as noise; -use rand::Rng; use rand::seq::SliceRandom; use std::{io, task::Poll}; -use util::TestHandler; - -type TestNetwork = Network; -type TestTransport = transport::Boxed<(PeerId, StreamMuxerBox)>; - -fn new_network(cfg: NetworkConfig) -> TestNetwork { - let local_key = identity::Keypair::generate_ed25519(); - let local_public_key = local_key.public(); - let noise_keys = noise::Keypair::::new().into_authentic(&local_key).unwrap(); - let transport: TestTransport = libp2p_tcp::TcpConfig::new() - .upgrade(upgrade::Version::V1) - .authenticate(noise::NoiseConfig::xx(noise_keys).into_authenticated()) - .multiplex(libp2p_mplex::MplexConfig::new()) - .boxed() - .and_then(|(peer, mplex), _| { - // Gracefully close the connection to allow protocol - // negotiation to complete. - util::CloseMuxer::new(mplex).map_ok(move |mplex| (peer, mplex)) - }) - .boxed(); - TestNetwork::new(transport, local_public_key.into(), cfg) -} +use util::{TestHandler, test_network}; #[test] fn deny_incoming_connec() { // Checks whether refusing an incoming connection on a swarm triggers the correct events. - let mut swarm1 = new_network(NetworkConfig::default()); - let mut swarm2 = new_network(NetworkConfig::default()); + let mut swarm1 = test_network(NetworkConfig::default()); + let mut swarm2 = test_network(NetworkConfig::default()); swarm1.listen_on("/ip4/127.0.0.1/tcp/0".parse().unwrap()).unwrap(); @@ -121,7 +92,7 @@ fn dial_self() { // // The last two can happen in any order. - let mut swarm = new_network(NetworkConfig::default()); + let mut swarm = test_network(NetworkConfig::default()); swarm.listen_on("/ip4/127.0.0.1/tcp/0".parse().unwrap()).unwrap(); let (local_address, mut swarm) = async_std::task::block_on( @@ -180,7 +151,7 @@ fn dial_self() { fn dial_self_by_id() { // Trying to dial self by passing the same `PeerId` shouldn't even be possible in the first // place. - let mut swarm = new_network(NetworkConfig::default()); + let mut swarm = test_network(NetworkConfig::default()); let peer_id = swarm.local_peer_id().clone(); assert!(swarm.peer(peer_id).into_disconnected().is_none()); } @@ -189,7 +160,7 @@ fn dial_self_by_id() { fn multiple_addresses_err() { // Tries dialing multiple addresses, and makes sure there's one dialing error per address. - let mut swarm = new_network(NetworkConfig::default()); + let mut swarm = test_network(NetworkConfig::default()); let mut addresses = Vec::new(); for _ in 0 .. 3 { @@ -233,44 +204,3 @@ fn multiple_addresses_err() { } })).unwrap(); } - -#[test] -fn connection_limit() { - let outgoing_per_peer_limit = rand::thread_rng().gen_range(1, 10); - let outgoing_limit = 2 * outgoing_per_peer_limit; - - let mut cfg = NetworkConfig::default(); - cfg.set_outgoing_per_peer_limit(outgoing_per_peer_limit); - cfg.set_outgoing_limit(outgoing_limit); - let mut network = new_network(cfg); - - let target = PeerId::random(); - for _ in 0 .. outgoing_per_peer_limit { - network.peer(target.clone()) - .dial(Multiaddr::empty(), Vec::new(), TestHandler()) - .ok() - .expect("Unexpected connection limit."); - } - - let err = network.peer(target) - .dial(Multiaddr::empty(), Vec::new(), TestHandler()) - .expect_err("Unexpected dialing success."); - - assert_eq!(err.current, outgoing_per_peer_limit); - assert_eq!(err.limit, outgoing_per_peer_limit); - - let target2 = PeerId::random(); - for _ in outgoing_per_peer_limit .. outgoing_limit { - network.peer(target2.clone()) - .dial(Multiaddr::empty(), Vec::new(), TestHandler()) - .ok() - .expect("Unexpected connection limit."); - } - - let err = network.peer(target2) - .dial(Multiaddr::empty(), Vec::new(), TestHandler()) - .expect_err("Unexpected dialing success."); - - assert_eq!(err.current, outgoing_limit); - assert_eq!(err.limit, outgoing_limit); -} diff --git a/core/tests/util.rs b/core/tests/util.rs index 306728d618ff..c20a2c59305e 100644 --- a/core/tests/util.rs +++ b/core/tests/util.rs @@ -4,16 +4,42 @@ use futures::prelude::*; use libp2p_core::{ Multiaddr, + PeerId, + Transport, connection::{ ConnectionHandler, ConnectionHandlerEvent, Substream, SubstreamEndpoint, }, + identity, muxing::{StreamMuxer, StreamMuxerBox}, + network::{Network, NetworkConfig}, + transport, + upgrade, }; +use libp2p_mplex as mplex; +use libp2p_noise as noise; +use libp2p_tcp as tcp; use std::{io, pin::Pin, task::Context, task::Poll}; +type TestNetwork = Network; +type TestTransport = transport::Boxed<(PeerId, StreamMuxerBox)>; + +/// Creates a new `TestNetwork` with a TCP transport. +pub fn test_network(cfg: NetworkConfig) -> TestNetwork { + let local_key = identity::Keypair::generate_ed25519(); + let local_public_key = local_key.public(); + let noise_keys = noise::Keypair::::new().into_authentic(&local_key).unwrap(); + let transport: TestTransport = tcp::TcpConfig::new() + .upgrade(upgrade::Version::V1) + .authenticate(noise::NoiseConfig::xx(noise_keys).into_authenticated()) + .multiplex(mplex::MplexConfig::new()) + .boxed(); + + TestNetwork::new(transport, local_public_key.into(), cfg) +} + pub struct TestHandler(); impl ConnectionHandler for TestHandler { @@ -35,7 +61,7 @@ impl ConnectionHandler for TestHandler { fn poll(&mut self, _: &mut Context<'_>) -> Poll, Self::Error>> { - Poll::Ready(Ok(ConnectionHandlerEvent::Custom(()))) + Poll::Pending } } diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md index 734b39144819..639641e75cfe 100644 --- a/swarm/CHANGELOG.md +++ b/swarm/CHANGELOG.md @@ -1,5 +1,8 @@ # 0.25.0 [unreleased] +- Changed parameters for connection limits from `usize` to `u32`. + Connection limits are now configured via `SwarmBuilder::connection_limits()`. + - Update `libp2p-core`. - Expose configurable scores for external addresses, as well as diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index f9009675416b..b3e0d4deeaf5 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -113,6 +113,7 @@ use libp2p_core::{ transport::{self, TransportError}, muxing::StreamMuxerBox, network::{ + ConnectionLimits, Network, NetworkInfo, NetworkEvent, @@ -987,7 +988,7 @@ where TBehaviour: NetworkBehaviour, /// By default, unless another executor has been configured, /// [`SwarmBuilder::build`] will try to set up a `ThreadPool`. pub fn executor(mut self, e: Box) -> Self { - self.network_config.set_executor(e); + self.network_config = self.network_config.with_executor(e); self } @@ -1001,7 +1002,7 @@ where TBehaviour: NetworkBehaviour, /// be sleeping more often than necessary. Increasing this value increases /// the overall memory usage. pub fn notify_handler_buffer_size(mut self, n: NonZeroUsize) -> Self { - self.network_config.set_notify_handler_buffer_size(n); + self.network_config = self.network_config.with_notify_handler_buffer_size(n); self } @@ -1029,28 +1030,13 @@ where TBehaviour: NetworkBehaviour, /// event is emitted and the moment when it is received by the /// [`NetworkBehaviour`]. pub fn connection_event_buffer_size(mut self, n: usize) -> Self { - self.network_config.set_connection_event_buffer_size(n); + self.network_config = self.network_config.with_connection_event_buffer_size(n); self } - /// Configures a limit for the number of simultaneous incoming - /// connection attempts. - pub fn incoming_connection_limit(mut self, n: usize) -> Self { - self.network_config.set_incoming_limit(n); - self - } - - /// Configures a limit for the number of simultaneous outgoing - /// connection attempts. - pub fn outgoing_connection_limit(mut self, n: usize) -> Self { - self.network_config.set_outgoing_limit(n); - self - } - - /// Configures a limit for the number of simultaneous - /// established connections per peer. - pub fn peer_connection_limit(mut self, n: usize) -> Self { - self.network_config.set_established_per_peer_limit(n); + /// Configures the connection limits. + pub fn connection_limits(mut self, limits: ConnectionLimits) -> Self { + self.network_config = self.network_config.with_connection_limits(limits); self } @@ -1064,20 +1050,21 @@ where TBehaviour: NetworkBehaviour, .map(|info| info.protocol_name().to_vec()) .collect(); - let mut network_cfg = self.network_config; - // If no executor has been explicitly configured, try to set up a thread pool. - if network_cfg.executor().is_none() { + let network_cfg = self.network_config.or_else_with_executor(|| { match ThreadPoolBuilder::new() .name_prefix("libp2p-swarm-task-") .create() { Ok(tp) => { - network_cfg.set_executor(Box::new(move |f| tp.spawn_ok(f))); + Some(Box::new(move |f| tp.spawn_ok(f))) }, - Err(err) => log::warn!("Failed to create executor thread pool: {:?}", err) + Err(err) => { + log::warn!("Failed to create executor thread pool: {:?}", err); + None + } } - } + }); let network = Network::new(self.transport, self.local_peer_id, network_cfg); From 5e21806a3842c68bc33fd31b23c7ddc849fc006a Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 24 Nov 2020 12:40:11 +0100 Subject: [PATCH 04/19] Some petty changes head of QUIC (#1854) --- LICENSE | 2 +- core/src/lib.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/LICENSE b/LICENSE index d4bb412c336f..da84696303f4 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright 2017 Parity Technologies (UK) Ltd. +Copyright 2017-2020 Parity Technologies (UK) Ltd. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in diff --git a/core/src/lib.rs b/core/src/lib.rs index e2836a1e3d13..844fd2a23bc5 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -23,14 +23,14 @@ //! The main concepts of libp2p-core are: //! //! - A [`PeerId`] is a unique global identifier for a node on the network. -//! Each node must have a different `PeerId`. Normally, a `PeerId` is the +//! Each node must have a different [`PeerId`]. Normally, a [`PeerId`] is the //! hash of the public key used to negotiate encryption on the //! communication channel, thereby guaranteeing that they cannot be spoofed. //! - The [`Transport`] trait defines how to reach a remote node or listen for -//! incoming remote connections. See the `transport` module. +//! incoming remote connections. See the [`transport`] module. //! - The [`StreamMuxer`] trait is implemented on structs that hold a connection //! to a remote and can subdivide this connection into multiple substreams. -//! See the `muxing` module. +//! See the [`muxing`] module. //! - The [`UpgradeInfo`], [`InboundUpgrade`] and [`OutboundUpgrade`] traits //! define how to upgrade each individual substream to use a protocol. //! See the `upgrade` module. From e3af8b7d03bdf7e4b156481102a6a961c23a8946 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Tue, 24 Nov 2020 15:59:22 +0100 Subject: [PATCH 05/19] *: Apply clippy suggestions and enable clippy on CI (#1850) * *: Apply clippy suggestions * .github: Add clippy check * protocols/kad/record: Implement custom PartialEq for ProviderRecord --- .github/workflows/ci.yml | 36 +++++++++++++++++++++++ core/src/connection.rs | 12 ++------ core/src/connection/listeners.rs | 2 -- core/src/connection/manager.rs | 6 +--- core/src/connection/pool.rs | 26 +++++++--------- core/src/network.rs | 13 ++++---- core/src/network/peer.rs | 17 +++++------ core/src/peer_id.rs | 1 - core/src/transport.rs | 24 +++------------ core/src/transport/memory.rs | 2 +- core/src/transport/upgrade.rs | 2 +- misc/multiaddr/src/errors.rs | 7 +---- misc/multiaddr/src/lib.rs | 6 ++++ misc/multiaddr/src/protocol.rs | 1 - misc/multistream-select/src/lib.rs | 3 -- misc/multistream-select/src/negotiated.rs | 27 +++++++---------- misc/multistream-select/src/protocol.rs | 9 +++--- muxers/mplex/src/codec.rs | 1 + protocols/kad/CHANGELOG.md | 3 ++ protocols/kad/src/record.rs | 15 ++++++++-- protocols/mdns/src/service.rs | 2 +- protocols/pnet/src/lib.rs | 4 +-- 22 files changed, 108 insertions(+), 111 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2943bd6cfb6a..d36164843d3e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -109,6 +109,42 @@ jobs: - name: Check rustdoc links run: RUSTDOCFLAGS="--deny broken_intra_doc_links" cargo doc --verbose --workspace --no-deps --document-private-items + check-clippy: + runs-on: ubuntu-latest + steps: + + - name: Cancel Previous Runs + uses: styfle/cancel-workflow-action@0.6.0 + with: + access_token: ${{ github.token }} + + - uses: actions/checkout@v2 + + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + override: true + components: clippy + + - name: Cache CARGO_HOME + uses: actions/cache@v2 + with: + path: ~/.cargo + key: cargo-home-${{ hashFiles('Cargo.toml') }} + + - name: Cache cargo build + uses: actions/cache@v2 + with: + path: target + key: cargo-build-target-${{ hashFiles('Cargo.toml') }} + + - name: Run cargo clippy + uses: actions-rs/cargo@v1 + with: + command: clippy + args: -- -A clippy::mutable_key_type -A clippy::type_complexity + integration-test: name: Integration tests runs-on: ubuntu-latest diff --git a/core/src/connection.rs b/core/src/connection.rs index f64801b944c9..455a708b1fb4 100644 --- a/core/src/connection.rs +++ b/core/src/connection.rs @@ -63,20 +63,12 @@ impl std::ops::Not for Endpoint { impl Endpoint { /// Is this endpoint a dialer? pub fn is_dialer(self) -> bool { - if let Endpoint::Dialer = self { - true - } else { - false - } + matches!(self, Endpoint::Dialer) } /// Is this endpoint a listener? pub fn is_listener(self) -> bool { - if let Endpoint::Listener = self { - true - } else { - false - } + matches!(self, Endpoint::Listener) } } diff --git a/core/src/connection/listeners.rs b/core/src/connection/listeners.rs index dab7b4fb6d62..962c14a7b0dc 100644 --- a/core/src/connection/listeners.rs +++ b/core/src/connection/listeners.rs @@ -41,7 +41,6 @@ use std::{collections::VecDeque, fmt, pin::Pin}; /// # Example /// /// ```no_run -/// # fn main() { /// use futures::prelude::*; /// use libp2p_core::connection::{ListenersEvent, ListenersStream}; /// @@ -75,7 +74,6 @@ use std::{collections::VecDeque, fmt, pin::Pin}; /// } /// } /// }) -/// # } /// ``` pub struct ListenersStream where diff --git a/core/src/connection/manager.rs b/core/src/connection/manager.rs index e505556f34b8..50550f123c21 100644 --- a/core/src/connection/manager.rs +++ b/core/src/connection/manager.rs @@ -336,10 +336,7 @@ impl Manager { /// Checks whether an established connection with the given ID is currently managed. pub fn is_established(&self, id: &ConnectionId) -> bool { - match self.tasks.get(&id.0) { - Some(TaskInfo { state: TaskState::Established(..), .. }) => true, - _ => false - } + matches!(self.tasks.get(&id.0), Some(TaskInfo { state: TaskState::Established(..), .. })) } /// Polls the manager for events relating to the managed connections. @@ -528,4 +525,3 @@ impl<'a, I> PendingEntry<'a, I> { self.task.remove(); } } - diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index e40c937cbf66..7705d441ae4b 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -457,15 +457,12 @@ impl // Count upwards because we push to / pop from the end. See also `Pool::poll`. let mut num_established = 0; for (&id, endpoint) in conns.iter() { - match self.manager.entry(id) { - Some(manager::Entry::Established(e)) => { - let connected = e.remove(); - self.disconnected.push(Disconnected { - id, connected, num_established - }); - num_established += 1; - }, - _ => {} + if let Some(manager::Entry::Established(e)) = self.manager.entry(id) { + let connected = e.remove(); + self.disconnected.push(Disconnected { + id, connected, num_established + }); + num_established += 1; } self.counters.dec_established(endpoint); } @@ -475,12 +472,9 @@ impl let mut aborted = Vec::new(); for (&id, (_endpoint, peer2)) in &self.pending { if Some(peer) == peer2.as_ref() { - match self.manager.entry(id) { - Some(manager::Entry::Pending(e)) => { - e.abort(); - aborted.push(id); - }, - _ => {} + if let Some(manager::Entry::Pending(e)) = self.manager.entry(id) { + e.abort(); + aborted.push(id); } } } @@ -578,7 +572,7 @@ impl // connections. Thus we `pop()` them off from the end to emit the // events in an order that properly counts down `num_established`. // See also `Pool::disconnect`. - while let Some(Disconnected { + if let Some(Disconnected { id, connected, num_established }) = self.disconnected.pop() { return Poll::Ready(PoolEvent::ConnectionClosed { diff --git a/core/src/network.rs b/core/src/network.rs index 59444662cf78..5819ba26be06 100644 --- a/core/src/network.rs +++ b/core/src/network.rs @@ -384,14 +384,11 @@ where let event = match self.pool.poll(cx) { Poll::Pending => return Poll::Pending, Poll::Ready(PoolEvent::ConnectionEstablished { connection, num_established }) => { - match self.dialing.entry(connection.peer_id().clone()) { - hash_map::Entry::Occupied(mut e) => { - e.get_mut().retain(|s| s.current.0 != connection.id()); - if e.get().is_empty() { - e.remove(); - } - }, - _ => {} + if let hash_map::Entry::Occupied(mut e) = self.dialing.entry(connection.peer_id().clone()) { + e.get_mut().retain(|s| s.current.0 != connection.id()); + if e.get().is_empty() { + e.remove(); + } } NetworkEvent::ConnectionEstablished { diff --git a/core/src/network/peer.rs b/core/src/network/peer.rs index 1f0d58ad4f0d..284ef4e44f92 100644 --- a/core/src/network/peer.rs +++ b/core/src/network/peer.rs @@ -197,10 +197,7 @@ where /// /// Returns `true` iff [`Peer::into_disconnected`] returns `Some`. pub fn is_disconnected(&self) -> bool { - match self { - Peer::Disconnected(..) => true, - _ => false - } + matches!(self, Peer::Disconnected(..)) } /// Initiates a new dialing attempt to this peer using the given addresses. @@ -303,8 +300,8 @@ where } /// Obtains an established connection to the peer by ID. - pub fn connection<'b>(&'b mut self, id: ConnectionId) - -> Option> + pub fn connection(&mut self, id: ConnectionId) + -> Option> { self.network.pool.get_established(id) } @@ -334,8 +331,8 @@ where } /// Gets an iterator over all established connections to the peer. - pub fn connections<'b>(&'b mut self) -> - EstablishedConnectionIter<'b, + pub fn connections(&mut self) -> + EstablishedConnectionIter< impl Iterator, TInEvent, TOutEvent, @@ -347,8 +344,8 @@ where } /// Obtains some established connection to the peer. - pub fn some_connection<'b>(&'b mut self) - -> EstablishedConnection<'b, TInEvent> + pub fn some_connection(&mut self) + -> EstablishedConnection { self.connections() .into_first() diff --git a/core/src/peer_id.rs b/core/src/peer_id.rs index 383e921571fa..b81eeefcfd34 100644 --- a/core/src/peer_id.rs +++ b/core/src/peer_id.rs @@ -20,7 +20,6 @@ use crate::PublicKey; use bytes::Bytes; -use bs58; use thiserror::Error; use multihash::{Code, Multihash, MultihashDigest}; use rand::Rng; diff --git a/core/src/transport.rs b/core/src/transport.rs index da0b75f17b6d..50499ec1b826 100644 --- a/core/src/transport.rs +++ b/core/src/transport.rs @@ -258,11 +258,7 @@ impl ListenerEvent { /// Returns `true` if this is an `Upgrade` listener event. pub fn is_upgrade(&self) -> bool { - if let ListenerEvent::Upgrade {..} = self { - true - } else { - false - } + matches!(self, ListenerEvent::Upgrade {..}) } /// Try to turn this listener event into upgrade parts. @@ -279,11 +275,7 @@ impl ListenerEvent { /// Returns `true` if this is a `NewAddress` listener event. pub fn is_new_address(&self) -> bool { - if let ListenerEvent::NewAddress(_) = self { - true - } else { - false - } + matches!(self, ListenerEvent::NewAddress(_)) } /// Try to turn this listener event into the `NewAddress` part. @@ -300,11 +292,7 @@ impl ListenerEvent { /// Returns `true` if this is an `AddressExpired` listener event. pub fn is_address_expired(&self) -> bool { - if let ListenerEvent::AddressExpired(_) = self { - true - } else { - false - } + matches!(self, ListenerEvent::AddressExpired(_)) } /// Try to turn this listener event into the `AddressExpired` part. @@ -321,11 +309,7 @@ impl ListenerEvent { /// Returns `true` if this is an `Error` listener event. pub fn is_error(&self) -> bool { - if let ListenerEvent::Error(_) = self { - true - } else { - false - } + matches!(self, ListenerEvent::Error(_)) } /// Try to turn this listener event into the `Error` part. diff --git a/core/src/transport/memory.rs b/core/src/transport/memory.rs index 69be07d42f01..e7d306302a86 100644 --- a/core/src/transport/memory.rs +++ b/core/src/transport/memory.rs @@ -101,7 +101,7 @@ pub struct DialFuture { impl DialFuture { fn new(port: NonZeroU64) -> Option { - let sender = HUB.get(&port)?.clone(); + let sender = HUB.get(&port)?; let (_dial_port_channel, dial_port) = HUB.register_port(0) .expect("there to be some random unoccupied port."); diff --git a/core/src/transport/upgrade.rs b/core/src/transport/upgrade.rs index 09f70b20e0ae..4304314b91f7 100644 --- a/core/src/transport/upgrade.rs +++ b/core/src/transport/upgrade.rs @@ -367,7 +367,7 @@ where type Dial = DialUpgradeFuture; fn dial(self, addr: Multiaddr) -> Result> { - let future = self.inner.dial(addr.clone()) + let future = self.inner.dial(addr) .map_err(|err| err.map(TransportUpgradeError::Transport))?; Ok(DialUpgradeFuture { future: Box::pin(future), diff --git a/misc/multiaddr/src/errors.rs b/misc/multiaddr/src/errors.rs index 245cea1c47ec..cb27e3d6acc5 100644 --- a/misc/multiaddr/src/errors.rs +++ b/misc/multiaddr/src/errors.rs @@ -1,12 +1,11 @@ use std::{net, fmt, error, io, num, str, string}; -use bs58; -use multihash; use unsigned_varint::decode; pub type Result = ::std::result::Result; /// Error types #[derive(Debug)] +#[non_exhaustive] pub enum Error { DataLessThanLen, InvalidMultiaddr, @@ -15,8 +14,6 @@ pub enum Error { ParsingError(Box), UnknownProtocolId(u32), UnknownProtocolString(String), - #[doc(hidden)] - __Nonexhaustive } impl fmt::Display for Error { @@ -29,7 +26,6 @@ impl fmt::Display for Error { Error::ParsingError(e) => write!(f, "failed to parse: {}", e), Error::UnknownProtocolId(id) => write!(f, "unknown protocol id: {}", id), Error::UnknownProtocolString(string) => write!(f, "unknown protocol string: {}", string), - Error::__Nonexhaustive => f.write_str("__Nonexhaustive") } } } @@ -92,4 +88,3 @@ impl From for Error { Error::InvalidUvar(e) } } - diff --git a/misc/multiaddr/src/lib.rs b/misc/multiaddr/src/lib.rs index cc63ddf2f564..0174e33c7541 100644 --- a/misc/multiaddr/src/lib.rs +++ b/misc/multiaddr/src/lib.rs @@ -40,6 +40,7 @@ static_assertions::const_assert! { } /// Representation of a Multiaddr. +#[allow(clippy::rc_buffer)] #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Hash)] pub struct Multiaddr { bytes: Arc> } @@ -59,6 +60,11 @@ impl Multiaddr { self.bytes.len() } + /// Returns true if the length of this multiaddress is 0. + pub fn is_empty(&self) -> bool { + self.bytes.len() == 0 + } + /// Return a copy of this [`Multiaddr`]'s byte representation. pub fn to_vec(&self) -> Vec { Vec::from(&self.bytes[..]) diff --git a/misc/multiaddr/src/protocol.rs b/misc/multiaddr/src/protocol.rs index 149554524805..d1a5ab9562ea 100644 --- a/misc/multiaddr/src/protocol.rs +++ b/misc/multiaddr/src/protocol.rs @@ -1,6 +1,5 @@ use arrayref::array_ref; -use bs58; use byteorder::{BigEndian, ByteOrder, ReadBytesExt, WriteBytesExt}; use crate::{Result, Error}; use data_encoding::BASE32; diff --git a/misc/multistream-select/src/lib.rs b/misc/multistream-select/src/lib.rs index abb088be6109..246a620a4bbb 100644 --- a/misc/multistream-select/src/lib.rs +++ b/misc/multistream-select/src/lib.rs @@ -76,7 +76,6 @@ //! For a dialer: //! //! ```no_run -//! # fn main() { //! use async_std::net::TcpStream; //! use multistream_select::{dialer_select_proto, Version}; //! use futures::prelude::*; @@ -90,7 +89,6 @@ //! println!("Negotiated protocol: {:?}", protocol); //! // You can now use `_io` to communicate with the remote. //! }); -//! # } //! ``` //! @@ -105,4 +103,3 @@ pub use self::negotiated::{Negotiated, NegotiatedComplete, NegotiationError}; pub use self::protocol::{ProtocolError, Version}; pub use self::dialer_select::{dialer_select_proto, DialerSelectFuture}; pub use self::listener_select::{listener_select_proto, ListenerSelectFuture}; - diff --git a/misc/multistream-select/src/negotiated.rs b/misc/multistream-select/src/negotiated.rs index a5c00cbcab5c..9e8f38efe2c9 100644 --- a/misc/multistream-select/src/negotiated.rs +++ b/misc/multistream-select/src/negotiated.rs @@ -61,12 +61,12 @@ where match Negotiated::poll(Pin::new(&mut io), cx) { Poll::Pending => { self.inner = Some(io); - return Poll::Pending + Poll::Pending }, Poll::Ready(Ok(())) => Poll::Ready(Ok(io)), Poll::Ready(Err(err)) => { self.inner = Some(io); - return Poll::Ready(Err(err)); + Poll::Ready(Err(err)) } } } @@ -104,9 +104,8 @@ impl Negotiated { let mut this = self.project(); - match this.state.as_mut().project() { - StateProj::Completed { .. } => return Poll::Ready(Ok(())), - _ => {} + if let StateProj::Completed { .. } = this.state.as_mut().project() { + return Poll::Ready(Ok(())); } // Read outstanding protocol negotiation messages. @@ -189,12 +188,9 @@ where -> Poll> { loop { - match self.as_mut().project().state.project() { - StateProj::Completed { io } => { - // If protocol negotiation is complete, commence with reading. - return io.poll_read(cx, buf) - }, - _ => {} + if let StateProj::Completed { io } = self.as_mut().project().state.project() { + // If protocol negotiation is complete, commence with reading. + return io.poll_read(cx, buf); } // Poll the `Negotiated`, driving protocol negotiation to completion, @@ -220,12 +216,9 @@ where -> Poll> { loop { - match self.as_mut().project().state.project() { - StateProj::Completed { io } => { - // If protocol negotiation is complete, commence with reading. - return io.poll_read_vectored(cx, bufs) - }, - _ => {} + if let StateProj::Completed { io } = self.as_mut().project().state.project() { + // If protocol negotiation is complete, commence with reading. + return io.poll_read_vectored(cx, bufs) } // Poll the `Negotiated`, driving protocol negotiation to completion, diff --git a/misc/multistream-select/src/protocol.rs b/misc/multistream-select/src/protocol.rs index 5248247f7ce0..4944e5f44590 100644 --- a/misc/multistream-select/src/protocol.rs +++ b/misc/multistream-select/src/protocol.rs @@ -240,7 +240,7 @@ impl Message { let mut remaining: &[u8] = &msg; loop { // A well-formed message must be terminated with a newline. - if remaining == &[b'\n'] { + if remaining == [b'\n'] { break } else if protocols.len() == MAX_PROTOCOLS { return Err(ProtocolError::TooManyProtocols) @@ -261,7 +261,7 @@ impl Message { remaining = &tail[len ..]; } - return Ok(Message::Protocols(protocols)); + Ok(Message::Protocols(protocols)) } } @@ -342,7 +342,7 @@ where Poll::Pending => Poll::Pending, Poll::Ready(None) => Poll::Ready(None), Poll::Ready(Some(Ok(m))) => Poll::Ready(Some(Ok(m))), - Poll::Ready(Some(Err(err))) => Poll::Ready(Some(Err(From::from(err)))), + Poll::Ready(Some(Err(err))) => Poll::Ready(Some(Err(err))), } } } @@ -450,7 +450,7 @@ impl Into for ProtocolError { if let ProtocolError::IoError(e) = self { return e } - return io::ErrorKind::InvalidData.into() + io::ErrorKind::InvalidData.into() } } @@ -529,4 +529,3 @@ mod tests { quickcheck(prop as fn(_)) } } - diff --git a/muxers/mplex/src/codec.rs b/muxers/mplex/src/codec.rs index 25d653bc0adb..b91865adc78e 100644 --- a/muxers/mplex/src/codec.rs +++ b/muxers/mplex/src/codec.rs @@ -61,6 +61,7 @@ impl fmt::Display for LocalStreamId { } impl Hash for LocalStreamId { + #![allow(clippy::derive_hash_xor_eq)] fn hash(&self, state: &mut H) { state.write_u32(self.num); } diff --git a/protocols/kad/CHANGELOG.md b/protocols/kad/CHANGELOG.md index 2d075e5e241e..1635c0ef7927 100644 --- a/protocols/kad/CHANGELOG.md +++ b/protocols/kad/CHANGELOG.md @@ -2,6 +2,9 @@ - Update `libp2p-core` and `libp2p-swarm`. +- Have two `ProviderRecord`s be equal iff their `key` and `provider` fields are + equal. [PR 1850](https://github.com/libp2p/rust-libp2p/pull/1850/). + # 0.25.0 [2020-11-09] - Upon newly established connections, delay routing table diff --git a/protocols/kad/src/record.rs b/protocols/kad/src/record.rs index a10240e62ec1..5a15fdd10343 100644 --- a/protocols/kad/src/record.rs +++ b/protocols/kad/src/record.rs @@ -103,7 +103,11 @@ impl Record { /// A record stored in the DHT whose value is the ID of a peer /// who can provide the value on-demand. -#[derive(Clone, Debug, PartialEq, Eq)] +/// +/// Note: Two [`ProviderRecord`]s as well as their corresponding hashes are +/// equal iff their `key` and `provider` fields are equal. See the [`Hash`] and +/// [`PartialEq`] implementations. +#[derive(Clone, Debug)] pub struct ProviderRecord { /// The key whose value is provided by the provider. pub key: Key, @@ -122,6 +126,14 @@ impl Hash for ProviderRecord { } } +impl PartialEq for ProviderRecord { + fn eq(&self, other: &Self) -> bool { + self.key == other.key && self.provider == other.provider + } +} + +impl Eq for ProviderRecord {} + impl ProviderRecord { /// Creates a new provider record for insertion into a `RecordStore`. pub fn new(key: K, provider: PeerId, addresses: Vec) -> Self @@ -187,4 +199,3 @@ mod tests { } } } - diff --git a/protocols/mdns/src/service.rs b/protocols/mdns/src/service.rs index 0d99c13dcb4e..7b1a8de226a9 100644 --- a/protocols/mdns/src/service.rs +++ b/protocols/mdns/src/service.rs @@ -551,7 +551,7 @@ impl MdnsPeer { MdnsPeer { addrs, - peer_id: my_peer_id.clone(), + peer_id: my_peer_id, ttl, } } diff --git a/protocols/pnet/src/lib.rs b/protocols/pnet/src/lib.rs index 0feb627aee10..f3d95905675a 100644 --- a/protocols/pnet/src/lib.rs +++ b/protocols/pnet/src/lib.rs @@ -74,7 +74,7 @@ impl PreSharedKey { cipher.apply_keystream(&mut enc); let mut hasher = Shake128::default(); hasher.write_all(&enc).expect("shake128 failed"); - hasher.finalize_xof().read(&mut out).expect("shake128 failed"); + hasher.finalize_xof().read_exact(&mut out).expect("shake128 failed"); Fingerprint(out) } } @@ -109,7 +109,7 @@ impl FromStr for PreSharedKey { type Err = KeyParseError; fn from_str(s: &str) -> Result { - if let &[keytype, encoding, key] = s.lines().take(3).collect::>().as_slice() { + if let [keytype, encoding, key] = *s.lines().take(3).collect::>().as_slice() { if keytype != "/key/swarm/psk/1.0.0/" { return Err(KeyParseError::InvalidKeyType); } From 86d228122284cd678dc431361ae544e7bdba1750 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Tue, 24 Nov 2020 17:36:44 +0100 Subject: [PATCH 06/19] Tweak docs of StreamMuxer::poll_event --- core/src/muxing.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/muxing.rs b/core/src/muxing.rs index 1b0b9749cd18..c8ae456fac2b 100644 --- a/core/src/muxing.rs +++ b/core/src/muxing.rs @@ -91,6 +91,9 @@ pub trait StreamMuxer { /// is ready to be polled, similar to the API of `Stream::poll()`. /// Only the latest task that was used to call this method may be notified. /// + /// It is permissible and common to use this method to perform background + /// work, such as processing incoming packets and polling timers. + /// /// An error can be generated if the connection has been closed. fn poll_event(&self, cx: &mut Context<'_>) -> Poll, Self::Error>>; From c01ac52745bbd0360076099a048183d93c123610 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 24 Nov 2020 19:08:44 +0100 Subject: [PATCH 07/19] Update webpki-roots requirement from 0.20 to 0.21 (#1851) * Update webpki-roots requirement from 0.20 to 0.21 Updates the requirements on [webpki-roots](https://github.com/ctz/webpki-roots) to permit the latest version. - [Release notes](https://github.com/ctz/webpki-roots/releases) - [Commits](https://github.com/ctz/webpki-roots/compare/v/0.20.0...v/0.21.0) Signed-off-by: dependabot[bot] * transports/websocket: Update changelog Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Max Inden --- transports/websocket/CHANGELOG.md | 2 +- transports/websocket/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/transports/websocket/CHANGELOG.md b/transports/websocket/CHANGELOG.md index 245a98ad1ca9..0b2cd0f599b6 100644 --- a/transports/websocket/CHANGELOG.md +++ b/transports/websocket/CHANGELOG.md @@ -1,6 +1,6 @@ # 0.26.0 [unreleased] -- Update `libp2p-core`. +- Update dependencies. # 0.25.0 [2020-11-09] diff --git a/transports/websocket/Cargo.toml b/transports/websocket/Cargo.toml index 1390af062a74..dfd1a4712702 100644 --- a/transports/websocket/Cargo.toml +++ b/transports/websocket/Cargo.toml @@ -21,7 +21,7 @@ rw-stream-sink = "0.2.0" soketto = { version = "0.4.1", features = ["deflate"] } url = "2.1" webpki = "0.21" -webpki-roots = "0.20" +webpki-roots = "0.21" [dev-dependencies] libp2p-tcp = { path = "../tcp", features = ["async-std"] } From 65c4bf88dca8df114bf92fe069060a646bc2246b Mon Sep 17 00:00:00 2001 From: Roman Borschel Date: Wed, 25 Nov 2020 10:21:02 +0100 Subject: [PATCH 08/19] [multistream] Make the lazy variant more interoperable. (#1855) * Make the lazy variant interoperable. The remaining optimisation for `V1Lazy` for a listener in the negotiation, whereby the listener delays flushing of the multistream version header, is hereby removed. The remaining effect of `V1Lazy` is only on the side of the dialer, which delays flushing of its singular protocol proposal in order to send it together with the first application data (or an attempt is made to read from the negotiated stream, which similarly triggers a flush of the protocol proposal). This permits `V1Lazy` dialers to be interoperable with `V1` listeners. The remaining theoretical pitfall whereby application data gets misinterpreted as another protocol proposal by a listener remains, however unlikely. `V1` remains the default, but we may eventually risk just making this lazy dialer flush a part of the default `V1` implementation, removing the dedicated `V1Lazy` version identifier. * Update CHANGELOG * Separate versions from mere header lines. Every multistream-select version maps to a specific header line, but there may be different variants of the same multistream-select version using the same header line, i.e. the same wire protocol. * Cleanup * Update misc/multistream-select/CHANGELOG.md --- misc/multistream-select/CHANGELOG.md | 10 +++ misc/multistream-select/src/dialer_select.rs | 29 ++++-- misc/multistream-select/src/lib.rs | 89 ++++++++++++++----- .../multistream-select/src/listener_select.rs | 22 ++--- misc/multistream-select/src/negotiated.rs | 27 +++--- misc/multistream-select/src/protocol.rs | 86 ++++-------------- protocols/kad/src/behaviour/test.rs | 4 +- 7 files changed, 141 insertions(+), 126 deletions(-) diff --git a/misc/multistream-select/CHANGELOG.md b/misc/multistream-select/CHANGELOG.md index e61fa9b9d686..3ec5fb8d52d1 100644 --- a/misc/multistream-select/CHANGELOG.md +++ b/misc/multistream-select/CHANGELOG.md @@ -1,5 +1,15 @@ # 0.9.0 [unreleased] +- Make the `V1Lazy` upgrade strategy more interoperable with `V1`. Specifically, + the listener now behaves identically with `V1` and `V1Lazy`. Furthermore, the + multistream-select protocol header is now also identical, making `V1` and `V1Lazy` + indistinguishable on the wire. The remaining central effect of `V1Lazy` is that the dialer, + if it only supports a single protocol in a negotiation, optimistically settles on that + protocol without immediately flushing the negotiation data (i.e. protocol proposal) + and without waiting for the corresponding confirmation before it is able to start + sending application data, expecting the used protocol to be confirmed with + the response. + - Fix the encoding and decoding of `ls` responses to be spec-compliant and interoperable with other implementations. For a clean upgrade, `0.8.4` must already be deployed. diff --git a/misc/multistream-select/src/dialer_select.rs b/misc/multistream-select/src/dialer_select.rs index 38111d99c258..733c4903e82d 100644 --- a/misc/multistream-select/src/dialer_select.rs +++ b/misc/multistream-select/src/dialer_select.rs @@ -20,8 +20,8 @@ //! Protocol negotiation strategies for the peer acting as the dialer. -use crate::{Negotiated, NegotiationError}; -use crate::protocol::{Protocol, ProtocolError, MessageIO, Message, Version}; +use crate::{Negotiated, NegotiationError, Version}; +use crate::protocol::{Protocol, ProtocolError, MessageIO, Message, HeaderLine}; use futures::{future::Either, prelude::*}; use std::{convert::TryFrom as _, iter, mem, pin::Pin, task::{Context, Poll}}; @@ -41,7 +41,7 @@ use std::{convert::TryFrom as _, iter, mem, pin::Pin, task::{Context, Poll}}; /// thus an inaccurate size estimate may result in a suboptimal choice. /// /// Within the scope of this library, a dialer always commits to a specific -/// multistream-select protocol [`Version`], whereas a listener always supports +/// multistream-select [`Version`], whereas a listener always supports /// all versions supported by this library. Frictionless multistream-select /// protocol upgrades may thus proceed by deployments with updated listeners, /// eventually followed by deployments of dialers choosing the newer protocol. @@ -181,11 +181,15 @@ where }, } - if let Err(err) = Pin::new(&mut io).start_send(Message::Header(*this.version)) { + let h = HeaderLine::from(*this.version); + if let Err(err) = Pin::new(&mut io).start_send(Message::Header(h)) { return Poll::Ready(Err(From::from(err))); } let protocol = this.protocols.next().ok_or(NegotiationError::Failed)?; + + // The dialer always sends the header and the first protocol + // proposal in one go for efficiency. *this.state = SeqState::SendProtocol { io, protocol }; } @@ -209,9 +213,14 @@ where } else { match this.version { Version::V1 => *this.state = SeqState::FlushProtocol { io, protocol }, + // This is the only effect that `V1Lazy` has compared to `V1`: + // Optimistically settling on the only protocol that + // the dialer supports for this negotiation. Notably, + // the dialer expects a regular `V1` response. Version::V1Lazy => { log::debug!("Dialer: Expecting proposed protocol: {}", p); - let io = Negotiated::expecting(io.into_reader(), p, *this.version); + let hl = HeaderLine::from(Version::V1Lazy); + let io = Negotiated::expecting(io.into_reader(), p, Some(hl)); return Poll::Ready(Ok((protocol, io))) } } @@ -242,7 +251,7 @@ where }; match msg { - Message::Header(v) if v == *this.version => { + Message::Header(v) if v == HeaderLine::from(*this.version) => { *this.state = SeqState::AwaitProtocol { io, protocol }; } Message::Protocol(ref p) if p.as_ref() == protocol.as_ref() => { @@ -318,7 +327,8 @@ where }, } - if let Err(err) = Pin::new(&mut io).start_send(Message::Header(*this.version)) { + let msg = Message::Header(HeaderLine::from(*this.version)); + if let Err(err) = Pin::new(&mut io).start_send(msg) { return Poll::Ready(Err(From::from(err))); } @@ -366,7 +376,7 @@ where }; match &msg { - Message::Header(v) if v == this.version => { + Message::Header(h) if h == &HeaderLine::from(*this.version) => { *this.state = ParState::RecvProtocols { io } } Message::Protocols(supported) => { @@ -395,9 +405,10 @@ where if let Err(err) = Pin::new(&mut io).start_send(Message::Protocol(p.clone())) { return Poll::Ready(Err(From::from(err))); } + log::debug!("Dialer: Expecting proposed protocol: {}", p); + let io = Negotiated::expecting(io.into_reader(), p, None); - let io = Negotiated::expecting(io.into_reader(), p, *this.version); return Poll::Ready(Ok((protocol, io))) } diff --git a/misc/multistream-select/src/lib.rs b/misc/multistream-select/src/lib.rs index 246a620a4bbb..087b2a2cb21b 100644 --- a/misc/multistream-select/src/lib.rs +++ b/misc/multistream-select/src/lib.rs @@ -48,28 +48,23 @@ //! //! ## [`Negotiated`](self::Negotiated) //! -//! When a dialer or listener participating in a negotiation settles -//! on a protocol to use, the [`DialerSelectFuture`] respectively -//! [`ListenerSelectFuture`] yields a [`Negotiated`](self::Negotiated) -//! I/O stream. -//! -//! Notably, when a `DialerSelectFuture` resolves to a `Negotiated`, it may not yet -//! have written the last negotiation message to the underlying I/O stream and may -//! still be expecting confirmation for that protocol, despite having settled on -//! a protocol to use. -//! -//! Similarly, when a `ListenerSelectFuture` resolves to a `Negotiated`, it may not -//! yet have sent the last negotiation message despite having settled on a protocol -//! proposed by the dialer that it supports. -//! -//! This behaviour allows both the dialer and the listener to send data -//! relating to the negotiated protocol together with the last negotiation -//! message(s), which, in the case of the dialer only supporting a single -//! protocol, results in 0-RTT negotiation. Note, however, that a dialer -//! that performs multiple 0-RTT negotiations in sequence for different -//! protocols layered on top of each other may trigger undesirable behaviour -//! for a listener not supporting one of the intermediate protocols. -//! See [`dialer_select_proto`](self::dialer_select_proto). +//! A `Negotiated` represents an I/O stream that has settled on a protocol +//! to use. By default, with [`Version::V1`], protocol negotiation is always +//! at least one dedicated round-trip message exchange, before application +//! data for the negotiated protocol can be sent by the dialer. There is +//! a variant [`Version::V1Lazy`] that permits 0-RTT negotiation if the +//! dialer only supports a single protocol. In that case, when a dialer +//! settles on a protocol to use, the [`DialerSelectFuture`] yields a +//! [`Negotiated`](self::Negotiated) I/O stream before the negotiation +//! data has been flushed. It is then expecting confirmation for that protocol +//! as the first messages read from the stream. This behaviour allows the dialer +//! to immediately send data relating to the negotiated protocol together with the +//! remaining negotiation message(s). Note, however, that a dialer that performs +//! multiple 0-RTT negotiations in sequence for different protocols layered on +//! top of each other may trigger undesirable behaviour for a listener not +//! supporting one of the intermediate protocols. See +//! [`dialer_select_proto`](self::dialer_select_proto) and the documentation +//! of [`Version::V1Lazy`] for further details. //! //! ## Examples //! @@ -100,6 +95,54 @@ mod protocol; mod tests; pub use self::negotiated::{Negotiated, NegotiatedComplete, NegotiationError}; -pub use self::protocol::{ProtocolError, Version}; +pub use self::protocol::ProtocolError; pub use self::dialer_select::{dialer_select_proto, DialerSelectFuture}; pub use self::listener_select::{listener_select_proto, ListenerSelectFuture}; + +/// Supported multistream-select versions. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Version { + /// Version 1 of the multistream-select protocol. See [1] and [2]. + /// + /// [1]: https://github.com/libp2p/specs/blob/master/connections/README.md#protocol-negotiation + /// [2]: https://github.com/multiformats/multistream-select + V1, + /// A "lazy" variant of version 1 that is identical on the wire but whereby + /// the dialer delays flushing protocol negotiation data in order to combine + /// it with initial application data, thus performing 0-RTT negotiation. + /// + /// This strategy is only applicable for the node with the role of "dialer" + /// in the negotiation and only if the dialer supports just a single + /// application protocol. In that case the dialer immedidately "settles" + /// on that protocol, buffering the negotiation messages to be sent + /// with the first round of application protocol data (or an attempt + /// is made to read from the `Negotiated` I/O stream). + /// + /// A listener will behave identically to `V1`. This ensures interoperability with `V1`. + /// Notably, it will immediately send the multistream header as well as the protocol + /// confirmation, resulting in multiple frames being sent on the underlying transport. + /// Nevertheless, if the listener supports the protocol that the dialer optimistically + /// settled on, it can be a 0-RTT negotiation. + /// + /// > **Note**: `V1Lazy` is specific to `rust-libp2p`. The wire protocol is identical to `V1` + /// > and generally interoperable with peers only supporting `V1`. Nevertheless, there is a + /// > pitfall that is rarely encountered: When nesting multiple protocol negotiations, the + /// > listener should either be known to support all of the dialer's optimistically chosen + /// > protocols or there is must be no intermediate protocol without a payload and none of + /// > the protocol payloads must have the potential for being mistaken for a multistream-select + /// > protocol message. This avoids rare edge-cases whereby the listener may not recognize + /// > upgrade boundaries and erroneously process a request despite not supporting one of + /// > the intermediate protocols that the dialer committed to. See [1] and [2]. + /// + /// [1]: https://github.com/multiformats/go-multistream/issues/20 + /// [2]: https://github.com/libp2p/rust-libp2p/pull/1212 + V1Lazy, + // Draft: https://github.com/libp2p/specs/pull/95 + // V2, +} + +impl Default for Version { + fn default() -> Self { + Version::V1 + } +} \ No newline at end of file diff --git a/misc/multistream-select/src/listener_select.rs b/misc/multistream-select/src/listener_select.rs index 04f232283691..4b76d04ea04d 100644 --- a/misc/multistream-select/src/listener_select.rs +++ b/misc/multistream-select/src/listener_select.rs @@ -22,7 +22,7 @@ //! in a multistream-select protocol negotiation. use crate::{Negotiated, NegotiationError}; -use crate::protocol::{Protocol, ProtocolError, MessageIO, Message, Version}; +use crate::protocol::{Protocol, ProtocolError, MessageIO, Message, HeaderLine}; use futures::prelude::*; use smallvec::SmallVec; @@ -81,7 +81,7 @@ where N: AsRef<[u8]> { RecvHeader { io: MessageIO }, - SendHeader { io: MessageIO, version: Version }, + SendHeader { io: MessageIO }, RecvMessage { io: MessageIO }, SendMessage { io: MessageIO, @@ -111,8 +111,10 @@ where match mem::replace(this.state, State::Done) { State::RecvHeader { mut io } => { match io.poll_next_unpin(cx) { - Poll::Ready(Some(Ok(Message::Header(version)))) => { - *this.state = State::SendHeader { io, version } + Poll::Ready(Some(Ok(Message::Header(h)))) => { + match h { + HeaderLine::V1 => *this.state = State::SendHeader { io } + } } Poll::Ready(Some(Ok(_))) => { return Poll::Ready(Err(ProtocolError::InvalidMessage.into())) @@ -129,24 +131,22 @@ where } } - State::SendHeader { mut io, version } => { + State::SendHeader { mut io } => { match Pin::new(&mut io).poll_ready(cx) { Poll::Pending => { - *this.state = State::SendHeader { io, version }; + *this.state = State::SendHeader { io }; return Poll::Pending }, Poll::Ready(Ok(())) => {}, Poll::Ready(Err(err)) => return Poll::Ready(Err(From::from(err))), } - if let Err(err) = Pin::new(&mut io).start_send(Message::Header(version)) { + let msg = Message::Header(HeaderLine::V1); + if let Err(err) = Pin::new(&mut io).start_send(msg) { return Poll::Ready(Err(From::from(err))); } - *this.state = match version { - Version::V1 => State::Flush { io, protocol: None }, - Version::V1Lazy => State::RecvMessage { io }, - } + *this.state = State::Flush { io, protocol: None }; } State::RecvMessage { mut io } => { diff --git a/misc/multistream-select/src/negotiated.rs b/misc/multistream-select/src/negotiated.rs index 9e8f38efe2c9..e80d579f2b42 100644 --- a/misc/multistream-select/src/negotiated.rs +++ b/misc/multistream-select/src/negotiated.rs @@ -18,7 +18,7 @@ // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. -use crate::protocol::{Protocol, MessageReader, Message, Version, ProtocolError}; +use crate::protocol::{Protocol, MessageReader, Message, ProtocolError, HeaderLine}; use futures::{prelude::*, io::{IoSlice, IoSliceMut}, ready}; use pin_project::pin_project; @@ -80,8 +80,12 @@ impl Negotiated { /// Creates a `Negotiated` in state [`State::Expecting`] that is still /// expecting confirmation of the given `protocol`. - pub(crate) fn expecting(io: MessageReader, protocol: Protocol, version: Version) -> Self { - Negotiated { state: State::Expecting { io, protocol, version } } + pub(crate) fn expecting( + io: MessageReader, + protocol: Protocol, + header: Option + ) -> Self { + Negotiated { state: State::Expecting { io, protocol, header } } } /// Polls the `Negotiated` for completion. @@ -111,11 +115,11 @@ impl Negotiated { // Read outstanding protocol negotiation messages. loop { match mem::replace(&mut *this.state, State::Invalid) { - State::Expecting { mut io, protocol, version } => { + State::Expecting { mut io, header, protocol } => { let msg = match Pin::new(&mut io).poll_next(cx)? { Poll::Ready(Some(msg)) => msg, Poll::Pending => { - *this.state = State::Expecting { io, protocol, version }; + *this.state = State::Expecting { io, header, protocol }; return Poll::Pending }, Poll::Ready(None) => { @@ -124,9 +128,9 @@ impl Negotiated { } }; - if let Message::Header(v) = &msg { - if *v == version { - *this.state = State::Expecting { io, protocol, version }; + if let Message::Header(h) = &msg { + if Some(h) == header.as_ref() { + *this.state = State::Expecting { io, protocol, header: None }; continue } } @@ -165,10 +169,11 @@ enum State { /// The underlying I/O stream. #[pin] io: MessageReader, - /// The expected protocol (i.e. name and version). + /// The expected negotiation header/preamble (i.e. multistream-select version), + /// if one is still expected to be received. + header: Option, + /// The expected application protocol (i.e. name and version). protocol: Protocol, - /// The expected multistream-select protocol version. - version: Version }, /// In this state, a protocol has been agreed upon and I/O diff --git a/misc/multistream-select/src/protocol.rs b/misc/multistream-select/src/protocol.rs index 4944e5f44590..1d056de75ec6 100644 --- a/misc/multistream-select/src/protocol.rs +++ b/misc/multistream-select/src/protocol.rs @@ -25,6 +25,7 @@ //! `Stream` and `Sink` implementations of `MessageIO` and //! `MessageReader`. +use crate::Version; use crate::length_delimited::{LengthDelimited, LengthDelimitedReader}; use bytes::{Bytes, BytesMut, BufMut}; @@ -37,71 +38,25 @@ const MAX_PROTOCOLS: usize = 1000; /// The encoded form of a multistream-select 1.0.0 header message. const MSG_MULTISTREAM_1_0: &[u8] = b"/multistream/1.0.0\n"; -/// The encoded form of a multistream-select 1.0.0 header message. -const MSG_MULTISTREAM_1_0_LAZY: &[u8] = b"/multistream-lazy/1\n"; /// The encoded form of a multistream-select 'na' message. const MSG_PROTOCOL_NA: &[u8] = b"na\n"; /// The encoded form of a multistream-select 'ls' message. const MSG_LS: &[u8] = b"ls\n"; -/// Supported multistream-select protocol versions. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum Version { - /// Version 1 of the multistream-select protocol. See [1] and [2]. - /// - /// [1]: https://github.com/libp2p/specs/blob/master/connections/README.md#protocol-negotiation - /// [2]: https://github.com/multiformats/multistream-select +/// The multistream-select header lines preceeding negotiation. +/// +/// Every [`Version`] has a corresponding header line. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum HeaderLine { + /// The `/multistream/1.0.0` header line. V1, - /// A lazy variant of version 1 that is identical on the wire but delays - /// sending of protocol negotiation data as much as possible. - /// - /// Delaying the sending of protocol negotiation data can result in - /// significantly fewer network roundtrips used for the negotiation, - /// up to 0-RTT negotiation. - /// - /// 0-RTT negotiation is achieved if the dialer supports only a single - /// application protocol. In that case the dialer immedidately settles - /// on that protocol, buffering the negotiation messages to be sent - /// with the first round of application protocol data (or an attempt - /// is made to read from the `Negotiated` I/O stream). - /// - /// A listener receiving a `V1Lazy` header will similarly delay sending - /// of the protocol confirmation. Though typically the listener will need - /// to read the request data before sending its response, thus triggering - /// sending of the protocol confirmation, which, in absence of additional - /// buffering on lower layers will result in at least two response frames - /// to be sent. - /// - /// `V1Lazy` is specific to `rust-libp2p`: While the wire protocol - /// is identical to `V1`, delayed sending of protocol negotiation frames - /// is only safe under the following assumptions: - /// - /// 1. The dialer is assumed to always send the first multistream-select - /// protocol message immediately after the multistream header, without - /// first waiting for confirmation of that header. Since the listener - /// delays sending the protocol confirmation, a deadlock situation may - /// otherwise occurs that is only resolved by a timeout. This assumption - /// is trivially satisfied if both peers support and use `V1Lazy`. - /// - /// 2. When nesting multiple protocol negotiations, the listener is either - /// known to support all of the dialer's optimistically chosen protocols - /// or there is no intermediate protocol without a payload and none of - /// the protocol payloads has the potential for being mistaken for a - /// multistream-select protocol message. This avoids rare edge-cases whereby - /// the listener may not recognize upgrade boundaries and erroneously - /// process a request despite not supporting one of the intermediate - /// protocols that the dialer committed to. See [1] and [2]. - /// - /// [1]: https://github.com/multiformats/go-multistream/issues/20 - /// [2]: https://github.com/libp2p/rust-libp2p/pull/1212 - V1Lazy, - // Draft: https://github.com/libp2p/specs/pull/95 - // V2, } -impl Default for Version { - fn default() -> Self { - Version::V1 +impl From for HeaderLine { + fn from(v: Version) -> HeaderLine { + match v { + Version::V1 | Version::V1Lazy => HeaderLine::V1, + } } } @@ -148,7 +103,7 @@ impl fmt::Display for Protocol { pub enum Message { /// A header message identifies the multistream-select protocol /// that the sender wishes to speak. - Header(Version), + Header(HeaderLine), /// A protocol message identifies a protocol request or acknowledgement. Protocol(Protocol), /// A message through which a peer requests the complete list of @@ -164,16 +119,11 @@ impl Message { /// Encodes a `Message` into its byte representation. pub fn encode(&self, dest: &mut BytesMut) -> Result<(), ProtocolError> { match self { - Message::Header(Version::V1) => { + Message::Header(HeaderLine::V1) => { dest.reserve(MSG_MULTISTREAM_1_0.len()); dest.put(MSG_MULTISTREAM_1_0); Ok(()) } - Message::Header(Version::V1Lazy) => { - dest.reserve(MSG_MULTISTREAM_1_0_LAZY.len()); - dest.put(MSG_MULTISTREAM_1_0_LAZY); - Ok(()) - } Message::Protocol(p) => { let len = p.0.as_ref().len() + 1; // + 1 for \n dest.reserve(len); @@ -209,12 +159,8 @@ impl Message { /// Decodes a `Message` from its byte representation. pub fn decode(mut msg: Bytes) -> Result { - if msg == MSG_MULTISTREAM_1_0_LAZY { - return Ok(Message::Header(Version::V1Lazy)) - } - if msg == MSG_MULTISTREAM_1_0 { - return Ok(Message::Header(Version::V1)) + return Ok(Message::Header(HeaderLine::V1)) } if msg == MSG_PROTOCOL_NA { @@ -506,7 +452,7 @@ mod tests { impl Arbitrary for Message { fn arbitrary(g: &mut G) -> Message { match g.gen_range(0, 5) { - 0 => Message::Header(Version::V1), + 0 => Message::Header(HeaderLine::V1), 1 => Message::NotAvailable, 2 => Message::ListProtocols, 3 => Message::Protocol(Protocol::arbitrary(g)), diff --git a/protocols/kad/src/behaviour/test.rs b/protocols/kad/src/behaviour/test.rs index 71fede97ea9c..81533a5b5b77 100644 --- a/protocols/kad/src/behaviour/test.rs +++ b/protocols/kad/src/behaviour/test.rs @@ -702,8 +702,8 @@ fn get_record_many() { .. })) => { assert_eq!(id, qid); - assert_eq!(records.len(), num_results); - assert_eq!(records.first().unwrap().record, record); + assert!(records.len() >= num_results); + assert!(records.into_iter().all(|r| r.record == record)); return Poll::Ready(()); } // Ignore any other event. From 83e87f76c02f3bf50b65281977916fe25cfacd14 Mon Sep 17 00:00:00 2001 From: Roman Borschel Date: Wed, 25 Nov 2020 14:26:49 +0100 Subject: [PATCH 09/19] [swarm] Permit configuration override for the substream upgrade protocol to use. (#1858) * Permit global configuration of a substream upgrade protocol override. * Revert temporary test. * Revert temporary test. * Update swarm changelog. --- swarm/CHANGELOG.md | 4 ++ swarm/src/lib.rs | 43 +++++++++++++++++---- swarm/src/protocols_handler/node_handler.rs | 22 ++++++++++- 3 files changed, 60 insertions(+), 9 deletions(-) diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md index 639641e75cfe..0b7573997d5a 100644 --- a/swarm/CHANGELOG.md +++ b/swarm/CHANGELOG.md @@ -1,5 +1,9 @@ # 0.25.0 [unreleased] +- Permit a configuration override for the substream upgrade protocol + to use for all (outbound) substreams. + [PR 1858](https://github.com/libp2p/rust-libp2p/pull/1858). + - Changed parameters for connection limits from `usize` to `u32`. Connection limits are now configured via `SwarmBuilder::connection_limits()`. diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index b3e0d4deeaf5..af3ccb71bcba 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -120,7 +120,7 @@ use libp2p_core::{ NetworkConfig, peer::ConnectedPeer, }, - upgrade::ProtocolName, + upgrade::{ProtocolName}, }; use registry::{Addresses, AddressIntoIter}; use smallvec::SmallVec; @@ -286,7 +286,10 @@ where /// Pending event to be delivered to connection handlers /// (or dropped if the peer disconnected) before the `behaviour` /// can be polled again. - pending_event: Option<(PeerId, PendingNotifyHandler, TInEvent)> + pending_event: Option<(PeerId, PendingNotifyHandler, TInEvent)>, + + /// The configured override for substream protocol upgrades, if any. + substream_upgrade_protocol_override: Option, } impl Deref for @@ -357,8 +360,10 @@ where TBehaviour: NetworkBehaviour, /// Initiates a new dialing attempt to the given address. pub fn dial_addr(me: &mut Self, addr: Multiaddr) -> Result<(), ConnectionLimit> { - let handler = me.behaviour.new_handler(); - me.network.dial(&addr, handler.into_node_handler_builder()).map(|_id| ()) + let handler = me.behaviour.new_handler() + .into_node_handler_builder() + .with_substream_upgrade_protocol_override(me.substream_upgrade_protocol_override); + me.network.dial(&addr, handler).map(|_id| ()) } /// Initiates a new dialing attempt to the given peer. @@ -375,7 +380,9 @@ where TBehaviour: NetworkBehaviour, let result = if let Some(first) = addrs.next() { - let handler = me.behaviour.new_handler().into_node_handler_builder(); + let handler = me.behaviour.new_handler() + .into_node_handler_builder() + .with_substream_upgrade_protocol_override(me.substream_upgrade_protocol_override); me.network.peer(peer_id.clone()) .dial(first, addrs, handler) .map(|_| ()) @@ -546,10 +553,12 @@ where TBehaviour: NetworkBehaviour, }); }, Poll::Ready(NetworkEvent::IncomingConnection { connection, .. }) => { - let handler = this.behaviour.new_handler(); + let handler = this.behaviour.new_handler() + .into_node_handler_builder() + .with_substream_upgrade_protocol_override(this.substream_upgrade_protocol_override); let local_addr = connection.local_addr.clone(); let send_back_addr = connection.send_back_addr.clone(); - if let Err(e) = this.network.accept(connection, handler.into_node_handler_builder()) { + if let Err(e) = this.network.accept(connection, handler) { log::warn!("Incoming connection rejected: {:?}", e); } return Poll::Ready(SwarmEvent::IncomingConnection { @@ -962,6 +971,7 @@ pub struct SwarmBuilder { transport: transport::Boxed<(PeerId, StreamMuxerBox)>, behaviour: TBehaviour, network_config: NetworkConfig, + substream_upgrade_protocol_override: Option, } impl SwarmBuilder @@ -980,6 +990,7 @@ where TBehaviour: NetworkBehaviour, transport: transport, behaviour, network_config: Default::default(), + substream_upgrade_protocol_override: None, } } @@ -1040,6 +1051,21 @@ where TBehaviour: NetworkBehaviour, self } + /// Configures an override for the substream upgrade protocol to use. + /// + /// The subtream upgrade protocol is the multistream-select protocol + /// used for protocol negotiation on substreams. Since a listener + /// supports all existing versions, the choice of upgrade protocol + /// only effects the "dialer", i.e. the peer opening a substream. + /// + /// > **Note**: If configured, specific upgrade protocols for + /// > individual [`SubstreamProtocol`]s emitted by the `NetworkBehaviour` + /// > are ignored. + pub fn substream_upgrade_protocol_override(mut self, v: libp2p_core::upgrade::Version) -> Self { + self.substream_upgrade_protocol_override = Some(v); + self + } + /// Builds a `Swarm` with the current configuration. pub fn build(mut self) -> Swarm { let supported_protocols = self.behaviour @@ -1075,7 +1101,8 @@ where TBehaviour: NetworkBehaviour, listened_addrs: SmallVec::new(), external_addrs: Addresses::default(), banned_peers: HashSet::new(), - pending_event: None + pending_event: None, + substream_upgrade_protocol_override: self.substream_upgrade_protocol_override, } } } diff --git a/swarm/src/protocols_handler/node_handler.rs b/swarm/src/protocols_handler/node_handler.rs index a0d2f53d6c99..ce4b6b313fb3 100644 --- a/swarm/src/protocols_handler/node_handler.rs +++ b/swarm/src/protocols_handler/node_handler.rs @@ -49,6 +49,8 @@ use wasm_timer::{Delay, Instant}; pub struct NodeHandlerWrapperBuilder { /// The underlying handler. handler: TIntoProtoHandler, + /// The substream upgrade protocol override, if any. + substream_upgrade_protocol_override: Option, } impl NodeHandlerWrapperBuilder @@ -59,8 +61,17 @@ where pub(crate) fn new(handler: TIntoProtoHandler) -> Self { NodeHandlerWrapperBuilder { handler, + substream_upgrade_protocol_override: None, } } + + pub(crate) fn with_substream_upgrade_protocol_override( + mut self, + version: Option + ) -> Self { + self.substream_upgrade_protocol_override = version; + self + } } impl IntoConnectionHandler @@ -79,6 +90,7 @@ where queued_dial_upgrades: Vec::new(), unique_dial_upgrade_id: 0, shutdown: Shutdown::None, + substream_upgrade_protocol_override: self.substream_upgrade_protocol_override, } } } @@ -109,6 +121,8 @@ where unique_dial_upgrade_id: u64, /// The currently planned connection & handler shutdown. shutdown: Shutdown, + /// The substream upgrade protocol override, if any. + substream_upgrade_protocol_override: Option, } struct SubstreamUpgrade { @@ -254,7 +268,13 @@ where } }; - let (_, (version, upgrade)) = self.queued_dial_upgrades.remove(pos); + let (_, (mut version, upgrade)) = self.queued_dial_upgrades.remove(pos); + if let Some(v) = self.substream_upgrade_protocol_override { + if v != version { + log::debug!("Substream upgrade protocol override: {:?} -> {:?}", version, v); + version = v; + } + } let upgrade = upgrade::apply_outbound(substream, upgrade, version); let timeout = Delay::new(timeout); self.negotiating_out.push(SubstreamUpgrade { From bc102bc923eea966d393dcde634eef68c2ef00bd Mon Sep 17 00:00:00 2001 From: Roman Borschel Date: Wed, 25 Nov 2020 15:30:13 +0100 Subject: [PATCH 10/19] Prepare release of `libp2p-v0.31` and all unreleased sub-crates. (#1857) * Prepare v0.31 * Update core-derive * Update minimum multiaddr patch version in libp2p crate. --- CHANGELOG.md | 2 +- Cargo.toml | 4 ++-- core/CHANGELOG.md | 2 +- misc/core-derive/CHANGELOG.md | 4 ++++ misc/core-derive/Cargo.toml | 2 +- misc/multistream-select/CHANGELOG.md | 2 +- muxers/mplex/CHANGELOG.md | 2 +- muxers/yamux/CHANGELOG.md | 2 +- protocols/deflate/CHANGELOG.md | 2 +- protocols/floodsub/CHANGELOG.md | 2 +- protocols/gossipsub/CHANGELOG.md | 2 +- protocols/identify/CHANGELOG.md | 2 +- protocols/kad/CHANGELOG.md | 2 +- protocols/mdns/CHANGELOG.md | 2 +- protocols/noise/CHANGELOG.md | 2 +- protocols/ping/CHANGELOG.md | 2 +- protocols/plaintext/CHANGELOG.md | 2 +- protocols/request-response/CHANGELOG.md | 2 +- protocols/secio/CHANGELOG.md | 2 +- swarm/CHANGELOG.md | 2 +- transports/dns/CHANGELOG.md | 2 +- transports/tcp/CHANGELOG.md | 2 +- transports/uds/CHANGELOG.md | 2 +- transports/wasm-ext/CHANGELOG.md | 2 +- transports/websocket/CHANGELOG.md | 2 +- 25 files changed, 29 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e408bd9d46e..ed7dd3ff0d81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ - [`parity-multiaddr` CHANGELOG](misc/multiaddr/CHANGELOG.md) - [`libp2p-core-derive` CHANGELOG](misc/core-derive/CHANGELOG.md) -# Version 0.31.0 [unreleased] +# Version 0.31.0 [2020-11-25] - Update `multistream-select` and all dependent crates. diff --git a/Cargo.toml b/Cargo.toml index 2c87e40cf61f..578f260a0116 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,7 +63,7 @@ bytes = "0.5" futures = "0.3.1" lazy_static = "1.2" libp2p-core = { version = "0.25.0", path = "core" } -libp2p-core-derive = { version = "0.20.2", path = "misc/core-derive" } +libp2p-core-derive = { version = "0.21.0", path = "misc/core-derive" } libp2p-floodsub = { version = "0.25.0", path = "protocols/floodsub", optional = true } libp2p-gossipsub = { version = "0.25.0", path = "./protocols/gossipsub", optional = true } libp2p-identify = { version = "0.25.0", path = "protocols/identify", optional = true } @@ -78,7 +78,7 @@ libp2p-swarm = { version = "0.25.0", path = "swarm" } libp2p-uds = { version = "0.25.0", path = "transports/uds", optional = true } libp2p-wasm-ext = { version = "0.25.0", path = "transports/wasm-ext", optional = true } libp2p-yamux = { version = "0.28.0", path = "muxers/yamux", optional = true } -multiaddr = { package = "parity-multiaddr", version = "0.9.4", path = "misc/multiaddr" } +multiaddr = { package = "parity-multiaddr", version = "0.9.6", path = "misc/multiaddr" } parking_lot = "0.11.0" pin-project = "1.0.0" smallvec = "1.0" diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index 071218bc5715..a47c146ef718 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - The `NetworkConfig` API is now a builder that moves `self`. [PR 1848](https://github.com/libp2p/rust-libp2p/pull/1848/). diff --git a/misc/core-derive/CHANGELOG.md b/misc/core-derive/CHANGELOG.md index 4de00efd7c11..a41b0946724d 100644 --- a/misc/core-derive/CHANGELOG.md +++ b/misc/core-derive/CHANGELOG.md @@ -1,3 +1,7 @@ +# 0.21.0 [2020-11-25] + +- Update for compatibility with `libp2p-swarm-0.25`. + # 0.20.2 [2020-07-28] - Generate fully-qualified method name for `poll` to avoid diff --git a/misc/core-derive/Cargo.toml b/misc/core-derive/Cargo.toml index 03b64062de3e..a3077d6aa8a9 100644 --- a/misc/core-derive/Cargo.toml +++ b/misc/core-derive/Cargo.toml @@ -2,7 +2,7 @@ name = "libp2p-core-derive" edition = "2018" description = "Procedural macros of libp2p-core" -version = "0.20.2" +version = "0.21.0" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/misc/multistream-select/CHANGELOG.md b/misc/multistream-select/CHANGELOG.md index 3ec5fb8d52d1..1eec4776a21e 100644 --- a/misc/multistream-select/CHANGELOG.md +++ b/misc/multistream-select/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.9.0 [unreleased] +# 0.9.0 [2020-11-25] - Make the `V1Lazy` upgrade strategy more interoperable with `V1`. Specifically, the listener now behaves identically with `V1` and `V1Lazy`. Furthermore, the diff --git a/muxers/mplex/CHANGELOG.md b/muxers/mplex/CHANGELOG.md index bd0a51fe2c6a..2a1126104d7b 100644 --- a/muxers/mplex/CHANGELOG.md +++ b/muxers/mplex/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-core`. diff --git a/muxers/yamux/CHANGELOG.md b/muxers/yamux/CHANGELOG.md index 4ced071971da..a499d470f2d3 100644 --- a/muxers/yamux/CHANGELOG.md +++ b/muxers/yamux/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.28.0 [unreleased] +# 0.28.0 [2020-11-25] - Update `libp2p-core`. diff --git a/protocols/deflate/CHANGELOG.md b/protocols/deflate/CHANGELOG.md index 34d0b1962d82..b0f3e1760e53 100644 --- a/protocols/deflate/CHANGELOG.md +++ b/protocols/deflate/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-core`. diff --git a/protocols/floodsub/CHANGELOG.md b/protocols/floodsub/CHANGELOG.md index 696e02cd7331..927dfe608f4d 100644 --- a/protocols/floodsub/CHANGELOG.md +++ b/protocols/floodsub/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-swarm` and `libp2p-core`. diff --git a/protocols/gossipsub/CHANGELOG.md b/protocols/gossipsub/CHANGELOG.md index 7efabaa8c79b..fc01624c3279 100644 --- a/protocols/gossipsub/CHANGELOG.md +++ b/protocols/gossipsub/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-swarm` and `libp2p-core`. diff --git a/protocols/identify/CHANGELOG.md b/protocols/identify/CHANGELOG.md index 03761672e801..db7796a29bc2 100644 --- a/protocols/identify/CHANGELOG.md +++ b/protocols/identify/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-swarm` and `libp2p-core`. diff --git a/protocols/kad/CHANGELOG.md b/protocols/kad/CHANGELOG.md index 1635c0ef7927..4c42e37d1f78 100644 --- a/protocols/kad/CHANGELOG.md +++ b/protocols/kad/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.26.0 [unreleased] +# 0.26.0 [2020-11-25] - Update `libp2p-core` and `libp2p-swarm`. diff --git a/protocols/mdns/CHANGELOG.md b/protocols/mdns/CHANGELOG.md index 1d2f6d5915a6..3cfe9154619a 100644 --- a/protocols/mdns/CHANGELOG.md +++ b/protocols/mdns/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-swarm` and `libp2p-core`. diff --git a/protocols/noise/CHANGELOG.md b/protocols/noise/CHANGELOG.md index 851a70228573..29a6e0cbbc13 100644 --- a/protocols/noise/CHANGELOG.md +++ b/protocols/noise/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.27.0 [unreleased] +# 0.27.0 [2020-11-25] - Update `libp2p-core`. diff --git a/protocols/ping/CHANGELOG.md b/protocols/ping/CHANGELOG.md index 3f75fb1d3101..a86557c4b9fc 100644 --- a/protocols/ping/CHANGELOG.md +++ b/protocols/ping/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-swarm` and `libp2p-core`. diff --git a/protocols/plaintext/CHANGELOG.md b/protocols/plaintext/CHANGELOG.md index c6857f6341cb..635ae3cee982 100644 --- a/protocols/plaintext/CHANGELOG.md +++ b/protocols/plaintext/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-core`. diff --git a/protocols/request-response/CHANGELOG.md b/protocols/request-response/CHANGELOG.md index 198f1e6bc092..ed0d5d0e5e62 100644 --- a/protocols/request-response/CHANGELOG.md +++ b/protocols/request-response/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.6.0 [unreleased] +# 0.6.0 [2020-11-25] - Update `libp2p-swarm` and `libp2p-core`. diff --git a/protocols/secio/CHANGELOG.md b/protocols/secio/CHANGELOG.md index 6147c159f984..3c24d7664c27 100644 --- a/protocols/secio/CHANGELOG.md +++ b/protocols/secio/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-core`. diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md index 0b7573997d5a..1a8d7d8261d9 100644 --- a/swarm/CHANGELOG.md +++ b/swarm/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Permit a configuration override for the substream upgrade protocol to use for all (outbound) substreams. diff --git a/transports/dns/CHANGELOG.md b/transports/dns/CHANGELOG.md index 5a324eeaeaab..af69d4c0d624 100644 --- a/transports/dns/CHANGELOG.md +++ b/transports/dns/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-core`. diff --git a/transports/tcp/CHANGELOG.md b/transports/tcp/CHANGELOG.md index 4df28deb8c06..737c8b8e0a0d 100644 --- a/transports/tcp/CHANGELOG.md +++ b/transports/tcp/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-core`. diff --git a/transports/uds/CHANGELOG.md b/transports/uds/CHANGELOG.md index 3efa3d1c4eca..f9df580ec3c9 100644 --- a/transports/uds/CHANGELOG.md +++ b/transports/uds/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-core`. diff --git a/transports/wasm-ext/CHANGELOG.md b/transports/wasm-ext/CHANGELOG.md index 54c4ef132589..03429ba533cf 100644 --- a/transports/wasm-ext/CHANGELOG.md +++ b/transports/wasm-ext/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.25.0 [unreleased] +# 0.25.0 [2020-11-25] - Update `libp2p-core`. diff --git a/transports/websocket/CHANGELOG.md b/transports/websocket/CHANGELOG.md index 0b2cd0f599b6..f388f7acde1e 100644 --- a/transports/websocket/CHANGELOG.md +++ b/transports/websocket/CHANGELOG.md @@ -1,4 +1,4 @@ -# 0.26.0 [unreleased] +# 0.26.0 [2020-11-25] - Update dependencies. From 6f7acc0f411c4964f836defac02d4a8c73c60138 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Wed, 25 Nov 2020 16:42:37 +0100 Subject: [PATCH 11/19] Add missing multiaddr update. As a result of the multihash upgrade. --- Cargo.toml | 2 +- core/Cargo.toml | 2 +- misc/multiaddr/CHANGELOG.md | 4 ++++ misc/multiaddr/Cargo.toml | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 578f260a0116..ef9e92d11fc5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -78,7 +78,7 @@ libp2p-swarm = { version = "0.25.0", path = "swarm" } libp2p-uds = { version = "0.25.0", path = "transports/uds", optional = true } libp2p-wasm-ext = { version = "0.25.0", path = "transports/wasm-ext", optional = true } libp2p-yamux = { version = "0.28.0", path = "muxers/yamux", optional = true } -multiaddr = { package = "parity-multiaddr", version = "0.9.6", path = "misc/multiaddr" } +multiaddr = { package = "parity-multiaddr", version = "0.10.0", path = "misc/multiaddr" } parking_lot = "0.11.0" pin-project = "1.0.0" smallvec = "1.0" diff --git a/core/Cargo.toml b/core/Cargo.toml index bd209e2301f3..a24c6ed2171b 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -21,7 +21,7 @@ futures-timer = "3" lazy_static = "1.2" libsecp256k1 = { version = "0.3.1", optional = true } log = "0.4" -multiaddr = { package = "parity-multiaddr", version = "0.9.4", path = "../misc/multiaddr" } +multiaddr = { package = "parity-multiaddr", version = "0.10.0", path = "../misc/multiaddr" } multihash = { version = "0.13", default-features = false, features = ["std", "multihash-impl", "identity", "sha2"] } multistream-select = { version = "0.9.0", path = "../misc/multistream-select" } parking_lot = "0.11.0" diff --git a/misc/multiaddr/CHANGELOG.md b/misc/multiaddr/CHANGELOG.md index 230e3212ddf6..712d03e7f439 100644 --- a/misc/multiaddr/CHANGELOG.md +++ b/misc/multiaddr/CHANGELOG.md @@ -1,3 +1,7 @@ +# 0.10.0 [2020-11-25] + +- Upgrade multihash to `0.13`. + # 0.9.6 [2020-11-17] - Move the `from_url` module and functionality behind the `url` feature, diff --git a/misc/multiaddr/Cargo.toml b/misc/multiaddr/Cargo.toml index f69ad5f1deae..9dded4b788bd 100644 --- a/misc/multiaddr/Cargo.toml +++ b/misc/multiaddr/Cargo.toml @@ -6,7 +6,7 @@ description = "Implementation of the multiaddr format" homepage = "https://github.com/libp2p/rust-libp2p" keywords = ["multiaddr", "ipfs"] license = "MIT" -version = "0.9.6" +version = "0.10.0" [features] default = ["url"] From 5e5c3226d85ecbf9ffddc3c9e139dfd674dfffa7 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Wed, 25 Nov 2020 16:51:07 +0100 Subject: [PATCH 12/19] Prepare core-0.25.1 and multiaddr-0.10. --- Cargo.toml | 2 +- core/CHANGELOG.md | 4 ++++ core/Cargo.toml | 2 +- misc/multiaddr/Cargo.toml | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ef9e92d11fc5..1dfdb4386789 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,7 +62,7 @@ atomic = "0.5.0" bytes = "0.5" futures = "0.3.1" lazy_static = "1.2" -libp2p-core = { version = "0.25.0", path = "core" } +libp2p-core = { version = "0.25.1", path = "core" } libp2p-core-derive = { version = "0.21.0", path = "misc/core-derive" } libp2p-floodsub = { version = "0.25.0", path = "protocols/floodsub", optional = true } libp2p-gossipsub = { version = "0.25.0", path = "./protocols/gossipsub", optional = true } diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index a47c146ef718..0471b1ddcf19 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -1,3 +1,7 @@ +# 0.25.1 [2020-11-25] + +- Add missing multiaddr upgrade. + # 0.25.0 [2020-11-25] - The `NetworkConfig` API is now a builder that moves `self`. diff --git a/core/Cargo.toml b/core/Cargo.toml index a24c6ed2171b..b1531aedb307 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -2,7 +2,7 @@ name = "libp2p-core" edition = "2018" description = "Core traits and structs of libp2p" -version = "0.25.0" +version = "0.25.1" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/misc/multiaddr/Cargo.toml b/misc/multiaddr/Cargo.toml index 9dded4b788bd..1eeb4ec9cdcf 100644 --- a/misc/multiaddr/Cargo.toml +++ b/misc/multiaddr/Cargo.toml @@ -16,7 +16,7 @@ arrayref = "0.3" bs58 = "0.4.0" byteorder = "1.3.1" data-encoding = "2.1" -multihash = { version = "0.13", default-features = false, features = ["std"] } +multihash = { version = "0.13", default-features = false, features = ["std", "multihash-impl", "identity"] } percent-encoding = "2.1.0" serde = "1.0.70" static_assertions = "1.1" From d3ce35291b8d6076f983657d4d56961d02de59b7 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Thu, 26 Nov 2020 10:42:33 +0100 Subject: [PATCH 13/19] Correct async-std bound for tcp. --- CHANGELOG.md | 4 ++++ Cargo.toml | 4 ++-- transports/tcp/CHANGELOG.md | 5 +++++ transports/tcp/Cargo.toml | 4 ++-- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed7dd3ff0d81..caa4ff74e02e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ - [`parity-multiaddr` CHANGELOG](misc/multiaddr/CHANGELOG.md) - [`libp2p-core-derive` CHANGELOG](misc/core-derive/CHANGELOG.md) +# Version 0.31.1 [2020-11-26] + +- Bump minimum `libp2p-tcp` patch version. + # Version 0.31.0 [2020-11-25] - Update `multistream-select` and all dependent crates. diff --git a/Cargo.toml b/Cargo.toml index 1dfdb4386789..2a80de41daa3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "libp2p" edition = "2018" description = "Peer-to-peer networking library" -version = "0.31.0" +version = "0.31.1" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" @@ -88,7 +88,7 @@ wasm-timer = "0.2.4" libp2p-deflate = { version = "0.25.0", path = "protocols/deflate", optional = true } libp2p-dns = { version = "0.25.0", path = "transports/dns", optional = true } libp2p-mdns = { version = "0.25.0", path = "protocols/mdns", optional = true } -libp2p-tcp = { version = "0.25.0", path = "transports/tcp", optional = true } +libp2p-tcp = { version = "0.25.1", path = "transports/tcp", optional = true } libp2p-websocket = { version = "0.26.0", path = "transports/websocket", optional = true } [dev-dependencies] diff --git a/transports/tcp/CHANGELOG.md b/transports/tcp/CHANGELOG.md index 737c8b8e0a0d..cf32e603aa9e 100644 --- a/transports/tcp/CHANGELOG.md +++ b/transports/tcp/CHANGELOG.md @@ -1,3 +1,8 @@ +# 0.25.1 [2020-11-26] + +- Lower `async-std` version to `1.6`, for compatibility + with other libp2p crates. + # 0.25.0 [2020-11-25] - Update `libp2p-core`. diff --git a/transports/tcp/Cargo.toml b/transports/tcp/Cargo.toml index d21f3ac7c5a0..9717c394e96f 100644 --- a/transports/tcp/Cargo.toml +++ b/transports/tcp/Cargo.toml @@ -2,7 +2,7 @@ name = "libp2p-tcp" edition = "2018" description = "TCP/IP transport protocol for libp2p" -version = "0.25.0" +version = "0.25.1" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" @@ -10,7 +10,7 @@ keywords = ["peer-to-peer", "libp2p", "networking"] categories = ["network-programming", "asynchronous"] [dependencies] -async-std = { version = "1.7.0", optional = true } +async-std = { version = "1.6.5", optional = true } futures = "0.3.1" futures-timer = "3.0" if-addrs = "0.6.4" From dae07b075b421fb0a33bbf85d91ad802e37845f9 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 26 Nov 2020 21:01:38 +0100 Subject: [PATCH 14/19] swarm: Add ExpandedSwarm::is_connected (#1862) Commit 335e55e6 removed the `ConnectionInfo` trait in favor of `PeerId`s. Commit 1bd013c8 removed `ExpandedSwarm::connection_info` as it would only return the `PeerId` that the caller is already aware of. One could use `ExpandedSwarm::connection_info` not only to retrieve the `ConnectionInfo` for a given peer, but also to check whether the underlying `Network` has a connection to the peer. This commit exposes the `is_connected` method on `Network` via `ExpandedSwarm` to check whether the `Network` has an established connection to a given peer. --- swarm/CHANGELOG.md | 5 +++++ swarm/Cargo.toml | 2 +- swarm/src/lib.rs | 5 +++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md index 1a8d7d8261d9..c0437d9eafc6 100644 --- a/swarm/CHANGELOG.md +++ b/swarm/CHANGELOG.md @@ -1,3 +1,8 @@ +# 0.25.1 [2020-11-26] + +- Add `ExpandedSwarm::is_connected`. + [PR 1862](https://github.com/libp2p/rust-libp2p/pull/1862). + # 0.25.0 [2020-11-25] - Permit a configuration override for the substream upgrade protocol diff --git a/swarm/Cargo.toml b/swarm/Cargo.toml index ca527aabe954..7c82ff67d670 100644 --- a/swarm/Cargo.toml +++ b/swarm/Cargo.toml @@ -2,7 +2,7 @@ name = "libp2p-swarm" edition = "2018" description = "The libp2p swarm" -version = "0.25.0" +version = "0.25.1" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index af3ccb71bcba..7b00cfbd46a6 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -463,6 +463,11 @@ where TBehaviour: NetworkBehaviour, me.banned_peers.remove(&peer_id); } + /// Checks whether the [`Network`] has an established connection to a peer. + pub fn is_connected(me: &Self, peer_id: &PeerId) -> bool { + me.network.is_connected(peer_id) + } + /// Returns the next event that happens in the `Swarm`. /// /// Includes events from the `NetworkBehaviour` but also events about the connections status. From afc5d0ef82c20bc6e08c6f42126532217751403b Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 30 Nov 2020 11:11:20 +0100 Subject: [PATCH 15/19] .github/workflow: Don't require nightly for doc link check (#1866) The `broken_intra_doc_links` lint stabilized with Rust `v1.48`, thus the special usage of `nightly` is not needed anymore in the pipeline step. --- .github/workflows/ci.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d36164843d3e..06d460502840 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -101,11 +101,6 @@ jobs: - uses: actions/checkout@v2 - - name: Install nightly Rust - # TODO: intra-doc links are available on nightly only - # see https://doc.rust-lang.org/nightly/rustdoc/lints.html#intra_doc_link_resolution_failure - run: rustup default nightly-2020-09-17 - - name: Check rustdoc links run: RUSTDOCFLAGS="--deny broken_intra_doc_links" cargo doc --verbose --workspace --no-deps --document-private-items From 2cd2e6d6540bbfcddc62b3826e9574d457682284 Mon Sep 17 00:00:00 2001 From: Roman Borschel Date: Mon, 30 Nov 2020 16:45:40 +0100 Subject: [PATCH 16/19] Address some minor clippy warnings. (#1868) --- core/src/connection/pool.rs | 2 +- core/src/network/peer.rs | 12 ++++++------ misc/multiaddr/src/from_url.rs | 27 ++++++++++++++------------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/core/src/connection/pool.rs b/core/src/connection/pool.rs index 7705d441ae4b..ccb89d6f7afd 100644 --- a/core/src/connection/pool.rs +++ b/core/src/connection/pool.rs @@ -842,7 +842,7 @@ where I: Iterator { /// Obtains the next connection, if any. - pub fn next<'b>(&'b mut self) -> Option> + pub fn next(&mut self) -> Option> { while let Some(id) = self.ids.next() { if self.pool.manager.is_established(&id) { // (*) diff --git a/core/src/network/peer.rs b/core/src/network/peer.rs index 284ef4e44f92..a1039334d44e 100644 --- a/core/src/network/peer.rs +++ b/core/src/network/peer.rs @@ -432,8 +432,8 @@ where /// Obtains a dialing attempt to the peer by connection ID of /// the current connection attempt. - pub fn attempt<'b>(&'b mut self, id: ConnectionId) - -> Option> + pub fn attempt(&mut self, id: ConnectionId) + -> Option> { if let hash_map::Entry::Occupied(attempts) = self.network.dialing.entry(self.peer_id.clone()) { if let Some(pos) = attempts.get().iter().position(|s| s.current.0 == id) { @@ -446,8 +446,8 @@ where } /// Gets an iterator over all dialing (i.e. pending outgoing) connections to the peer. - pub fn attempts<'b>(&'b mut self) - -> DialingAttemptIter<'b, + pub fn attempts(&mut self) + -> DialingAttemptIter<'_, TInEvent, TOutEvent, THandler, @@ -460,8 +460,8 @@ where /// Obtains some dialing connection to the peer. /// /// At least one dialing connection is guaranteed to exist on a `DialingPeer`. - pub fn some_attempt<'b>(&'b mut self) - -> DialingAttempt<'b, TInEvent> + pub fn some_attempt(&mut self) + -> DialingAttempt<'_, TInEvent> { self.attempts() .into_first() diff --git a/misc/multiaddr/src/from_url.rs b/misc/multiaddr/src/from_url.rs index cfafbef87b4b..fa5acfacc01b 100644 --- a/misc/multiaddr/src/from_url.rs +++ b/misc/multiaddr/src/from_url.rs @@ -82,13 +82,13 @@ fn from_url_inner_http_ws(url: url::Url, lossy: bool) -> std::result::Result std::result::Result unreachable!("We only call this function for one of the given schemes; qed") }; - if !lossy { - if !url.username().is_empty() || url.password().is_some() || - url.query().is_some() || url.fragment().is_some() - { - return Err(FromUrlErr::InformationLoss); - } + if !lossy && ( + !url.username().is_empty() || + url.password().is_some() || + url.query().is_some() || + url.fragment().is_some() + ) { + return Err(FromUrlErr::InformationLoss); } Ok(Multiaddr::from(protocol)) From 86402311fc50901f824cc628f1bb1af3bb89b7f2 Mon Sep 17 00:00:00 2001 From: Roman Borschel Date: Wed, 2 Dec 2020 16:40:58 +0100 Subject: [PATCH 17/19] [multistream-select] Listener conformity for failed negotiations. (#1871) * [multistream-select] Listener conformity for failed negotiations. When `V1Lazy` is used and the listener does not support the optimistic (and singular) proposal of the dialer, it currently happens that dialer and listener get a different outcome of the negotiation. The dialer eventually detects the failed negotiation as soon as it tries to read from the stream, but the listener either encounters an invalid message or unexpected premature EOF, depending on the payload that the dialer sent prematurely after its protocol proposal. In these cases the listener must be lenient and fail the negotiation "normally", i.e. not with a protocol violation or an I/O error. * Update misc/multistream-select/src/tests.rs Co-authored-by: Max Inden * Refine error handling. Only be lenient with garbage or sudden EOF when reading just after having sent a protocol rejection. * Update misc/multistream-select/src/listener_select.rs Co-authored-by: Max Inden Co-authored-by: Max Inden --- .../multistream-select/src/listener_select.rs | 43 +++++++++- misc/multistream-select/src/tests.rs | 79 ++++++++++++++++--- 2 files changed, 110 insertions(+), 12 deletions(-) diff --git a/misc/multistream-select/src/listener_select.rs b/misc/multistream-select/src/listener_select.rs index 4b76d04ea04d..70463fa1cfe5 100644 --- a/misc/multistream-select/src/listener_select.rs +++ b/misc/multistream-select/src/listener_select.rs @@ -57,7 +57,8 @@ where protocols: SmallVec::from_iter(protocols), state: State::RecvHeader { io: MessageIO::new(inner) - } + }, + last_sent_na: false, } } @@ -72,7 +73,14 @@ where // TODO: It would be nice if eventually N = Protocol, which has a // few more implications on the API. protocols: SmallVec<[(N, Protocol); 8]>, - state: State + state: State, + /// Whether the last message sent was a protocol rejection (i.e. `na\n`). + /// + /// If the listener reads garbage or EOF after such a rejection, + /// the dialer is likely using `V1Lazy` and negotiation must be + /// considered failed, but not with a protocol violation or I/O + /// error. + last_sent_na: bool, } enum State @@ -166,7 +174,30 @@ where *this.state = State::RecvMessage { io }; return Poll::Pending; } - Poll::Ready(Some(Err(err))) => return Poll::Ready(Err(From::from(err))), + Poll::Ready(Some(Err(err))) => { + if *this.last_sent_na { + // When we read garbage or EOF after having already rejected a + // protocol, the dialer is most likely using `V1Lazy` and has + // optimistically settled on this protocol, so this is really a + // failed negotiation, not a protocol violation. In this case + // the dialer also raises `NegotiationError::Failed` when finally + // reading the `N/A` response. + if let ProtocolError::InvalidMessage = &err { + log::trace!("Listener: Negotiation failed with invalid \ + message after protocol rejection."); + return Poll::Ready(Err(NegotiationError::Failed)) + } + if let ProtocolError::IoError(e) = &err { + if e.kind() == std::io::ErrorKind::UnexpectedEof { + log::trace!("Listener: Negotiation failed with EOF \ + after protocol rejection."); + return Poll::Ready(Err(NegotiationError::Failed)) + } + } + } + + return Poll::Ready(Err(From::from(err))) + } }; match msg { @@ -209,6 +240,12 @@ where Poll::Ready(Err(err)) => return Poll::Ready(Err(From::from(err))), } + if let Message::NotAvailable = &message { + *this.last_sent_na = true; + } else { + *this.last_sent_na = false; + } + if let Err(err) = Pin::new(&mut io).start_send(message) { return Poll::Ready(Err(From::from(err))); } diff --git a/misc/multistream-select/src/tests.rs b/misc/multistream-select/src/tests.rs index 50ab7ded016b..f03d1b1ff754 100644 --- a/misc/multistream-select/src/tests.rs +++ b/misc/multistream-select/src/tests.rs @@ -18,7 +18,7 @@ // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. -//! Contains the unit tests of the library. +//! Integration tests for protocol negotiation. #![cfg(test)] @@ -74,16 +74,23 @@ fn select_proto_basic() { async_std::task::block_on(run(Version::V1Lazy)); } +/// Tests the expected behaviour of failed negotiations. #[test] -fn no_protocol_found() { - async fn run(version: Version) { +fn negotiation_failed() { + let _ = env_logger::try_init(); + + async fn run(Test { + version, + listen_protos, + dial_protos, + dial_payload + }: Test) { let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let listener_addr = listener.local_addr().unwrap(); let server = async_std::task::spawn(async move { let connec = listener.accept().await.unwrap().0; - let protos = vec![b"/proto1", b"/proto2"]; - let io = match listener_select_proto(connec, protos).await { + let io = match listener_select_proto(connec, listen_protos).await { Ok((_, io)) => io, Err(NegotiationError::Failed) => return, Err(NegotiationError::ProtocolError(e)) => panic!("Unexpected protocol error {}", e), @@ -96,12 +103,15 @@ fn no_protocol_found() { let client = async_std::task::spawn(async move { let connec = TcpStream::connect(&listener_addr).await.unwrap(); - let protos = vec![b"/proto3", b"/proto4"]; - let io = match dialer_select_proto(connec, protos.into_iter(), version).await { + let mut io = match dialer_select_proto(connec, dial_protos.into_iter(), version).await { Err(NegotiationError::Failed) => return, Ok((_, io)) => io, Err(_) => panic!() }; + // The dialer may write a payload that is even sent before it + // got confirmation of the last proposed protocol, when `V1Lazy` + // is used. + io.write_all(&dial_payload).await.unwrap(); match io.complete().await { Err(NegotiationError::Failed) => {}, _ => panic!(), @@ -112,8 +122,59 @@ fn no_protocol_found() { client.await; } - async_std::task::block_on(run(Version::V1)); - async_std::task::block_on(run(Version::V1Lazy)); + /// Parameters for a single test run. + #[derive(Clone)] + struct Test { + version: Version, + listen_protos: Vec<&'static str>, + dial_protos: Vec<&'static str>, + dial_payload: Vec, + } + + // Disjunct combinations of listen and dial protocols to test. + // + // The choices here cover the main distinction between a single + // and multiple protocols. + let protos = vec!{ + (vec!["/proto1"], vec!["/proto2"]), + (vec!["/proto1", "/proto2"], vec!["/proto3", "/proto4"]), + }; + + // The payloads that the dialer sends after "successful" negotiation, + // which may be sent even before the dialer got protocol confirmation + // when `V1Lazy` is used. + // + // The choices here cover the specific situations that can arise with + // `V1Lazy` and which must nevertheless behave identically to `V1` w.r.t. + // the outcome of the negotiation. + let payloads = vec!{ + // No payload, in which case all versions should behave identically + // in any case, i.e. the baseline test. + vec![], + // With this payload and `V1Lazy`, the listener interprets the first + // `1` as a message length and encounters an invalid message (the + // second `1`). The listener is nevertheless expected to fail + // negotiation normally, just like with `V1`. + vec![1,1], + // With this payload and `V1Lazy`, the listener interprets the first + // `42` as a message length and encounters unexpected EOF trying to + // read a message of that length. The listener is nevertheless expected + // to fail negotiation normally, just like with `V1` + vec![42,1], + }; + + for (listen_protos, dial_protos) in protos { + for dial_payload in payloads.clone() { + for &version in &[Version::V1, Version::V1Lazy] { + async_std::task::block_on(run(Test { + version, + listen_protos: listen_protos.clone(), + dial_protos: dial_protos.clone(), + dial_payload: dial_payload.clone(), + })) + } + } + } } #[test] From 4bdb61be0d4d53af63c794e1ae7720b3ae5c6c91 Mon Sep 17 00:00:00 2001 From: "Roman S. Borschel" Date: Wed, 2 Dec 2020 16:41:59 +0100 Subject: [PATCH 18/19] Prepare multistream-select-0.9.1 --- CHANGELOG.md | 4 ++++ Cargo.toml | 4 ++-- core/CHANGELOG.md | 4 ++++ core/Cargo.toml | 4 ++-- misc/multistream-select/CHANGELOG.md | 6 ++++++ misc/multistream-select/Cargo.toml | 2 +- 6 files changed, 19 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index caa4ff74e02e..cc3f4ebbf525 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ - [`parity-multiaddr` CHANGELOG](misc/multiaddr/CHANGELOG.md) - [`libp2p-core-derive` CHANGELOG](misc/core-derive/CHANGELOG.md) +# Version 0.31.2 [2020-12-02] + +- Bump minimum `libp2p-core` patch version. + # Version 0.31.1 [2020-11-26] - Bump minimum `libp2p-tcp` patch version. diff --git a/Cargo.toml b/Cargo.toml index 2a80de41daa3..c275fa913fac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "libp2p" edition = "2018" description = "Peer-to-peer networking library" -version = "0.31.1" +version = "0.31.2" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" @@ -62,7 +62,7 @@ atomic = "0.5.0" bytes = "0.5" futures = "0.3.1" lazy_static = "1.2" -libp2p-core = { version = "0.25.1", path = "core" } +libp2p-core = { version = "0.25.2", path = "core" } libp2p-core-derive = { version = "0.21.0", path = "misc/core-derive" } libp2p-floodsub = { version = "0.25.0", path = "protocols/floodsub", optional = true } libp2p-gossipsub = { version = "0.25.0", path = "./protocols/gossipsub", optional = true } diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index 0471b1ddcf19..09e654d2a6ac 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -1,3 +1,7 @@ +# 0.25.2 [2020-12-02] + +- Require `multistream-select-0.9.1`. + # 0.25.1 [2020-11-25] - Add missing multiaddr upgrade. diff --git a/core/Cargo.toml b/core/Cargo.toml index b1531aedb307..ce2aba164be5 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -2,7 +2,7 @@ name = "libp2p-core" edition = "2018" description = "Core traits and structs of libp2p" -version = "0.25.1" +version = "0.25.2" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" @@ -23,7 +23,7 @@ libsecp256k1 = { version = "0.3.1", optional = true } log = "0.4" multiaddr = { package = "parity-multiaddr", version = "0.10.0", path = "../misc/multiaddr" } multihash = { version = "0.13", default-features = false, features = ["std", "multihash-impl", "identity", "sha2"] } -multistream-select = { version = "0.9.0", path = "../misc/multistream-select" } +multistream-select = { version = "0.9.1", path = "../misc/multistream-select" } parking_lot = "0.11.0" pin-project = "1.0.0" prost = "0.6.1" diff --git a/misc/multistream-select/CHANGELOG.md b/misc/multistream-select/CHANGELOG.md index 1eec4776a21e..d41a56200706 100644 --- a/misc/multistream-select/CHANGELOG.md +++ b/misc/multistream-select/CHANGELOG.md @@ -1,3 +1,9 @@ +# 0.9.1 [2020-12-02] + +- Ensure uniform outcomes for failed negotiations with both + `V1` and `V1Lazy`. + [PR 1871](https://github.com/libp2p/rust-libp2p/pull/1871) + # 0.9.0 [2020-11-25] - Make the `V1Lazy` upgrade strategy more interoperable with `V1`. Specifically, diff --git a/misc/multistream-select/Cargo.toml b/misc/multistream-select/Cargo.toml index 7482fc986a15..d042e55c7e7d 100644 --- a/misc/multistream-select/Cargo.toml +++ b/misc/multistream-select/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "multistream-select" description = "Multistream-select negotiation protocol for libp2p" -version = "0.9.0" +version = "0.9.1" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" From 505a17dfc2bb09704cd355ca20767b84b5a6857c Mon Sep 17 00:00:00 2001 From: David Craven Date: Thu, 3 Dec 2020 13:30:52 +0100 Subject: [PATCH 19/19] Adds support for handling interface changes to mdns behaviour. (#1830) * mdns: handle address changes. * Update examples. * Use async-io. * Fix tokio-chat. * Address review comments. * Update if-watch. * Poll interfaces correctly. * Use socket2 and remove wasm-time. * Update if-watch. * Update versions and changelogs. * Further changelog updates. Co-authored-by: Roman Borschel Co-authored-by: Roman S. Borschel --- CHANGELOG.md | 4 + Cargo.toml | 11 +- examples/chat-tokio.rs | 9 +- examples/chat.rs | 2 +- examples/distributed-key-value-store.rs | 2 +- examples/mdns-passive-discovery.rs | 2 +- protocols/mdns/CHANGELOG.md | 13 +++ protocols/mdns/Cargo.toml | 31 +++-- protocols/mdns/src/behaviour.rs | 87 ++++++-------- protocols/mdns/src/dns.rs | 4 +- protocols/mdns/src/lib.rs | 10 +- protocols/mdns/src/service.rs | 149 ++++++++++++------------ src/lib.rs | 4 +- 13 files changed, 161 insertions(+), 167 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc3f4ebbf525..8cfa61efc243 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ - [`parity-multiaddr` CHANGELOG](misc/multiaddr/CHANGELOG.md) - [`libp2p-core-derive` CHANGELOG](misc/core-derive/CHANGELOG.md) +# Version 0.32.0 [unreleased] + +- Update to `libp2p-mdns-0.26`. + # Version 0.31.2 [2020-12-02] - Bump minimum `libp2p-core` patch version. diff --git a/Cargo.toml b/Cargo.toml index c275fa913fac..04f5dafdb887 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "libp2p" edition = "2018" description = "Peer-to-peer networking library" -version = "0.31.2" +version = "0.32.0" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" @@ -17,7 +17,7 @@ default = [ "identify", "kad", "gossipsub", - "mdns-async-std", + "mdns", "mplex", "noise", "ping", @@ -37,8 +37,7 @@ floodsub = ["libp2p-floodsub"] identify = ["libp2p-identify"] kad = ["libp2p-kad"] gossipsub = ["libp2p-gossipsub"] -mdns-async-std = ["libp2p-mdns", "libp2p-mdns/async-std"] -mdns-tokio = ["libp2p-mdns", "libp2p-mdns/tokio"] +mdns = ["libp2p-mdns"] mplex = ["libp2p-mplex"] noise = ["libp2p-noise"] ping = ["libp2p-ping"] @@ -87,7 +86,7 @@ wasm-timer = "0.2.4" [target.'cfg(not(any(target_os = "emscripten", target_os = "wasi", target_os = "unknown")))'.dependencies] libp2p-deflate = { version = "0.25.0", path = "protocols/deflate", optional = true } libp2p-dns = { version = "0.25.0", path = "transports/dns", optional = true } -libp2p-mdns = { version = "0.25.0", path = "protocols/mdns", optional = true } +libp2p-mdns = { version = "0.26.0", path = "protocols/mdns", optional = true } libp2p-tcp = { version = "0.25.1", path = "transports/tcp", optional = true } libp2p-websocket = { version = "0.26.0", path = "transports/websocket", optional = true } @@ -125,4 +124,4 @@ members = [ [[example]] name = "chat-tokio" -required-features = ["tcp-tokio", "mdns-tokio"] +required-features = ["tcp-tokio", "mdns"] diff --git a/examples/chat-tokio.rs b/examples/chat-tokio.rs index 7d919dbbf863..15577ac6dfc2 100644 --- a/examples/chat-tokio.rs +++ b/examples/chat-tokio.rs @@ -46,8 +46,7 @@ use libp2p::{ core::upgrade, identity, floodsub::{self, Floodsub, FloodsubEvent}, - // `TokioMdns` is available through the `mdns-tokio` feature. - mdns::{TokioMdns, MdnsEvent}, + mdns::{Mdns, MdnsEvent}, mplex, noise, swarm::{NetworkBehaviourEventProcess, SwarmBuilder}, @@ -90,7 +89,7 @@ async fn main() -> Result<(), Box> { #[derive(NetworkBehaviour)] struct MyBehaviour { floodsub: Floodsub, - mdns: TokioMdns, + mdns: Mdns, } impl NetworkBehaviourEventProcess for MyBehaviour { @@ -122,7 +121,7 @@ async fn main() -> Result<(), Box> { // Create a Swarm to manage peers and events. let mut swarm = { - let mdns = TokioMdns::new()?; + let mdns = Mdns::new().await?; let mut behaviour = MyBehaviour { floodsub: Floodsub::new(peer_id.clone()), mdns, @@ -172,4 +171,4 @@ async fn main() -> Result<(), Box> { } } } -} \ No newline at end of file +} diff --git a/examples/chat.rs b/examples/chat.rs index ba82849da30d..67966e07fabb 100644 --- a/examples/chat.rs +++ b/examples/chat.rs @@ -121,7 +121,7 @@ fn main() -> Result<(), Box> { // Create a Swarm to manage peers and events let mut swarm = { - let mdns = Mdns::new()?; + let mdns = task::block_on(Mdns::new())?; let mut behaviour = MyBehaviour { floodsub: Floodsub::new(local_peer_id.clone()), mdns, diff --git a/examples/distributed-key-value-store.rs b/examples/distributed-key-value-store.rs index 5be91d290a51..6d5df32f998d 100644 --- a/examples/distributed-key-value-store.rs +++ b/examples/distributed-key-value-store.rs @@ -151,7 +151,7 @@ fn main() -> Result<(), Box> { // Create a Kademlia behaviour. let store = MemoryStore::new(local_peer_id.clone()); let kademlia = Kademlia::new(local_peer_id.clone(), store); - let mdns = Mdns::new()?; + let mdns = task::block_on(Mdns::new())?; let behaviour = MyBehaviour { kademlia, mdns }; Swarm::new(transport, behaviour, local_peer_id) }; diff --git a/examples/mdns-passive-discovery.rs b/examples/mdns-passive-discovery.rs index b3b4ab7848fe..a8f4323a4b1d 100644 --- a/examples/mdns-passive-discovery.rs +++ b/examples/mdns-passive-discovery.rs @@ -26,7 +26,7 @@ fn main() -> Result<(), Box> { // This example provides passive discovery of the libp2p nodes on the // network that send mDNS queries and answers. task::block_on(async move { - let mut service = MdnsService::new()?; + let mut service = MdnsService::new().await?; loop { let (srv, packet) = service.next().await; match packet { diff --git a/protocols/mdns/CHANGELOG.md b/protocols/mdns/CHANGELOG.md index 3cfe9154619a..16c58ca0f9e5 100644 --- a/protocols/mdns/CHANGELOG.md +++ b/protocols/mdns/CHANGELOG.md @@ -1,3 +1,16 @@ +# 0.26.0 [unreleased] + +- Detect interface changes and join the MDNS multicast + group on all interfaces as they become available. + [PR 1830](https://github.com/libp2p/rust-libp2p/pull/1830). + +- Replace the use of macros for abstracting over `tokio` + and `async-std` with the use of `async-io`. As a result + there may now be an additional reactor thread running + called `async-io` when using `tokio`, with the futures + still being polled by the `tokio` runtime. + [PR 1830](https://github.com/libp2p/rust-libp2p/pull/1830). + # 0.25.0 [2020-11-25] - Update `libp2p-swarm` and `libp2p-core`. diff --git a/protocols/mdns/Cargo.toml b/protocols/mdns/Cargo.toml index 168a341e7d35..d1af2fde6690 100644 --- a/protocols/mdns/Cargo.toml +++ b/protocols/mdns/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "libp2p-mdns" edition = "2018" -version = "0.25.0" +version = "0.26.0" description = "Implementation of the libp2p mDNS discovery method" authors = ["Parity Technologies "] license = "MIT" @@ -10,22 +10,21 @@ keywords = ["peer-to-peer", "libp2p", "networking"] categories = ["network-programming", "asynchronous"] [dependencies] -async-std = { version = "1.6.2", optional = true } -data-encoding = "2.0" -dns-parser = "0.8" -either = "1.5.3" -futures = "0.3.1" -lazy_static = "1.2" +async-io = "1.3.0" +data-encoding = "2.3.1" +dns-parser = "0.8.0" +futures = "0.3.8" +if-watch = "0.1.6" +lazy_static = "1.4.0" libp2p-core = { version = "0.25.0", path = "../../core" } libp2p-swarm = { version = "0.25.0", path = "../../swarm" } -log = "0.4" -net2 = "0.2" -rand = "0.7" -smallvec = "1.0" -tokio = { version = "0.3", default-features = false, features = ["net"], optional = true } -void = "1.0" -wasm-timer = "0.2.4" +log = "0.4.11" +rand = "0.7.3" +smallvec = "1.5.0" +socket2 = { version = "0.3.17", features = ["reuseport"] } +void = "1.0.2" [dev-dependencies] -if-addrs = "0.6.4" -tokio = { version = "0.3", default-features = false, features = ["rt", "rt-multi-thread"] } +async-std = "1.7.0" +if-addrs = "0.6.5" +tokio = { version = "0.3.4", default-features = false, features = ["rt", "rt-multi-thread"] } diff --git a/protocols/mdns/src/behaviour.rs b/protocols/mdns/src/behaviour.rs index b9b5649d20a2..f2e9ee524fda 100644 --- a/protocols/mdns/src/behaviour.rs +++ b/protocols/mdns/src/behaviour.rs @@ -18,7 +18,8 @@ // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. -use crate::service::{MdnsPacket, build_query_response, build_service_discovery_response}; +use crate::service::{MdnsPacket, MdnsService, build_query_response, build_service_discovery_response}; +use async_io::Timer; use futures::prelude::*; use libp2p_core::{ Multiaddr, @@ -34,21 +35,16 @@ use libp2p_swarm::{ ProtocolsHandler, protocols_handler::DummyProtocolsHandler }; -use log::warn; use smallvec::SmallVec; -use std::{cmp, fmt, io, iter, mem, pin::Pin, time::Duration, task::Context, task::Poll}; -use wasm_timer::{Delay, Instant}; +use std::{cmp, fmt, io, iter, mem, pin::Pin, time::{Duration, Instant}, task::Context, task::Poll}; const MDNS_RESPONSE_TTL: std::time::Duration = Duration::from_secs(5 * 60); -macro_rules! codegen { - ($feature_name:expr, $behaviour_name:ident, $maybe_busy_wrapper:ident, $service_name:ty) => { - /// A `NetworkBehaviour` for mDNS. Automatically discovers peers on the local network and adds /// them to the topology. -pub struct $behaviour_name { +pub struct Mdns { /// The inner service. - service: $maybe_busy_wrapper, + service: MdnsBusyWrapper, /// List of nodes that we have discovered, the address, and when their TTL expires. /// @@ -59,44 +55,44 @@ pub struct $behaviour_name { /// Future that fires when the TTL of at least one node in `discovered_nodes` expires. /// /// `None` if `discovered_nodes` is empty. - closest_expiration: Option, + closest_expiration: Option, } /// `MdnsService::next` takes ownership of `self`, returning a future that resolves with both itself /// and a `MdnsPacket` (similar to the old Tokio socket send style). The two states are thus `Free` /// with an `MdnsService` or `Busy` with a future returning the original `MdnsService` and an /// `MdnsPacket`. -enum $maybe_busy_wrapper { - Free($service_name), - Busy(Pin + Send>>), +enum MdnsBusyWrapper { + Free(MdnsService), + Busy(Pin + Send>>), Poisoned, } -impl fmt::Debug for $maybe_busy_wrapper { +impl fmt::Debug for MdnsBusyWrapper { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - $maybe_busy_wrapper::Free(service) => { - fmt.debug_struct("$maybe_busy_wrapper::Free") + Self::Free(service) => { + fmt.debug_struct("MdnsBusyWrapper::Free") .field("service", service) .finish() }, - $maybe_busy_wrapper::Busy(_) => { - fmt.debug_struct("$maybe_busy_wrapper::Busy") + Self::Busy(_) => { + fmt.debug_struct("MdnsBusyWrapper::Busy") .finish() } - $maybe_busy_wrapper::Poisoned => { - fmt.debug_struct("$maybe_busy_wrapper::Poisoned") + Self::Poisoned => { + fmt.debug_struct("MdnsBusyWrapper::Poisoned") .finish() } } } } -impl $behaviour_name { +impl Mdns { /// Builds a new `Mdns` behaviour. - pub fn new() -> io::Result<$behaviour_name> { - Ok($behaviour_name { - service: $maybe_busy_wrapper::Free(<$service_name>::new()?), + pub async fn new() -> io::Result { + Ok(Self { + service: MdnsBusyWrapper::Free(MdnsService::new().await?), discovered_nodes: SmallVec::new(), closest_expiration: None, }) @@ -113,7 +109,7 @@ impl $behaviour_name { } } -impl NetworkBehaviour for $behaviour_name { +impl NetworkBehaviour for Mdns { type ProtocolsHandler = DummyProtocolsHandler; type OutEvent = MdnsEvent; @@ -138,9 +134,9 @@ impl NetworkBehaviour for $behaviour_name { &mut self, _: PeerId, _: ConnectionId, - _ev: ::OutEvent, + ev: ::OutEvent, ) { - void::unreachable(_ev) + void::unreachable(ev) } fn poll( @@ -155,9 +151,8 @@ impl NetworkBehaviour for $behaviour_name { > { // Remove expired peers. if let Some(ref mut closest_expiration) = self.closest_expiration { - match Future::poll(Pin::new(closest_expiration), cx) { - Poll::Ready(Ok(())) => { - let now = Instant::now(); + match Pin::new(closest_expiration).poll(cx) { + Poll::Ready(now) => { let mut expired = SmallVec::<[(PeerId, Multiaddr); 4]>::new(); while let Some(pos) = self.discovered_nodes.iter().position(|(_, _, exp)| *exp < now) { let (peer_id, addr, _) = self.discovered_nodes.remove(pos); @@ -173,38 +168,37 @@ impl NetworkBehaviour for $behaviour_name { } }, Poll::Pending => (), - Poll::Ready(Err(err)) => warn!("timer has errored: {:?}", err), } } // Polling the mDNS service, and obtain the list of nodes discovered this round. let discovered = loop { - let service = mem::replace(&mut self.service, $maybe_busy_wrapper::Poisoned); + let service = mem::replace(&mut self.service, MdnsBusyWrapper::Poisoned); let packet = match service { - $maybe_busy_wrapper::Free(service) => { - self.service = $maybe_busy_wrapper::Busy(Box::pin(service.next())); + MdnsBusyWrapper::Free(service) => { + self.service = MdnsBusyWrapper::Busy(Box::pin(service.next())); continue; }, - $maybe_busy_wrapper::Busy(mut fut) => { + MdnsBusyWrapper::Busy(mut fut) => { match fut.as_mut().poll(cx) { Poll::Ready((service, packet)) => { - self.service = $maybe_busy_wrapper::Free(service); + self.service = MdnsBusyWrapper::Free(service); packet }, Poll::Pending => { - self.service = $maybe_busy_wrapper::Busy(fut); + self.service = MdnsBusyWrapper::Busy(fut); return Poll::Pending; } } }, - $maybe_busy_wrapper::Poisoned => panic!("Mdns poisoned"), + MdnsBusyWrapper::Poisoned => panic!("Mdns poisoned"), }; match packet { MdnsPacket::Query(query) => { // MaybeBusyMdnsService should always be Free. - if let $maybe_busy_wrapper::Free(ref mut service) = self.service { + if let MdnsBusyWrapper::Free(ref mut service) = self.service { let resp = build_query_response( query.query_id(), params.local_peer_id().clone(), @@ -256,7 +250,7 @@ impl NetworkBehaviour for $behaviour_name { }, MdnsPacket::ServiceDiscovery(disc) => { // MaybeBusyMdnsService should always be Free. - if let $maybe_busy_wrapper::Free(ref mut service) = self.service { + if let MdnsBusyWrapper::Free(ref mut service) = self.service { let resp = build_service_discovery_response( disc.query_id(), MDNS_RESPONSE_TTL, @@ -273,7 +267,7 @@ impl NetworkBehaviour for $behaviour_name { .fold(None, |exp, &(_, _, elem_exp)| { Some(exp.map(|exp| cmp::min(exp, elem_exp)).unwrap_or(elem_exp)) }) - .map(Delay::new_at); + .map(Timer::at); Poll::Ready(NetworkBehaviourAction::GenerateEvent(MdnsEvent::Discovered(DiscoveredAddrsIter { inner: discovered.into_iter(), @@ -281,7 +275,7 @@ impl NetworkBehaviour for $behaviour_name { } } -impl fmt::Debug for $behaviour_name { +impl fmt::Debug for Mdns { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt.debug_struct("Mdns") .field("service", &self.service) @@ -289,15 +283,6 @@ impl fmt::Debug for $behaviour_name { } } -}; -} - -#[cfg(feature = "async-std")] -codegen!("async-std", Mdns, MaybeBusyMdnsService, crate::service::MdnsService); - -#[cfg(feature = "tokio")] -codegen!("tokio", TokioMdns, MaybeBusyTokioMdnsService, crate::service::TokioMdnsService); - /// Event that can be produced by the `Mdns` behaviour. #[derive(Debug)] pub enum MdnsEvent { diff --git a/protocols/mdns/src/dns.rs b/protocols/mdns/src/dns.rs index 4ac965843cc8..81adcdc28c65 100644 --- a/protocols/mdns/src/dns.rs +++ b/protocols/mdns/src/dns.rs @@ -22,9 +22,7 @@ //! `dns_parser` library. use crate::{META_QUERY_SERVICE, SERVICE_NAME}; -use data_encoding; use libp2p_core::{Multiaddr, PeerId}; -use rand; use std::{borrow::Cow, cmp, error, fmt, str, time::Duration}; /// Maximum size of a DNS label as per RFC1035 @@ -226,7 +224,7 @@ fn segment_peer_id(peer_id: String) -> String { /// Combines and encodes a `PeerId` and service name for a DNS query. fn encode_peer_id(peer_id: &PeerId) -> Vec { - // DNS-safe encoding for the Peer ID + // DNS-safe encoding for the Peer ID let raw_peer_id = data_encoding::BASE32_DNSCURVE.encode(&peer_id.as_bytes()); // ensure we don't have any labels over 63 bytes long let encoded_peer_id = segment_peer_id(raw_peer_id); diff --git a/protocols/mdns/src/lib.rs b/protocols/mdns/src/lib.rs index 292bd01b13fa..e8e152b9c8e0 100644 --- a/protocols/mdns/src/lib.rs +++ b/protocols/mdns/src/lib.rs @@ -35,12 +35,10 @@ const SERVICE_NAME: &[u8] = b"_p2p._udp.local"; /// Hardcoded name of the service used for DNS-SD. const META_QUERY_SERVICE: &[u8] = b"_services._dns-sd._udp.local"; -#[cfg(feature = "async-std")] -pub use self::{behaviour::Mdns, service::MdnsService}; -#[cfg(feature = "tokio")] -pub use self::{behaviour::TokioMdns, service::TokioMdnsService}; - -pub use self::behaviour::MdnsEvent; +pub use crate::{ + behaviour::{Mdns, MdnsEvent}, + service::MdnsService, +}; mod behaviour; mod dns; diff --git a/protocols/mdns/src/service.rs b/protocols/mdns/src/service.rs index 7b1a8de226a9..1b4dcfa727dd 100644 --- a/protocols/mdns/src/service.rs +++ b/protocols/mdns/src/service.rs @@ -19,14 +19,15 @@ // DEALINGS IN THE SOFTWARE. use crate::{SERVICE_NAME, META_QUERY_SERVICE, dns}; +use async_io::{Async, Timer}; use dns_parser::{Packet, RData}; -use either::Either::{Left, Right}; -use futures::{future, prelude::*}; +use futures::{prelude::*, select}; +use if_watch::{IfEvent, IfWatcher}; +use lazy_static::lazy_static; use libp2p_core::{multiaddr::{Multiaddr, Protocol}, PeerId}; use log::warn; -use std::{convert::TryFrom as _, fmt, io, net::Ipv4Addr, net::SocketAddr, str, time::{Duration, Instant}}; -use wasm_timer::Interval; -use lazy_static::lazy_static; +use socket2::{Socket, Domain, Type}; +use std::{convert::TryFrom, fmt, io, net::{IpAddr, Ipv4Addr, UdpSocket, SocketAddr}, str, time::{Duration, Instant}}; pub use dns::{MdnsResponseError, build_query_response, build_service_discovery_response}; @@ -37,9 +38,6 @@ lazy_static! { )); } -macro_rules! codegen { - ($feature_name:expr, $service_name:ident, $udp_socket:ty, $udp_socket_from_std:tt) => { - /// A running service that discovers libp2p peers and responds to other libp2p peers' queries on /// the local network. /// @@ -71,10 +69,7 @@ macro_rules! codegen { /// # let my_peer_id = PeerId::from(identity::Keypair::generate_ed25519().public()); /// # let my_listened_addrs: Vec = vec![]; /// # async { -/// # #[cfg(feature = "async-std")] -/// # let mut service = libp2p_mdns::service::MdnsService::new().unwrap(); -/// # #[cfg(feature = "tokio")] -/// # let mut service = libp2p_mdns::service::TokioMdnsService::new().unwrap(); +/// # let mut service = libp2p_mdns::service::MdnsService::new().await.unwrap(); /// let _future_to_poll = async { /// let (mut service, packet) = service.next().await; /// @@ -108,19 +103,18 @@ macro_rules! codegen { /// }; /// # }; /// # } -#[cfg_attr(docsrs, doc(cfg(feature = $feature_name)))] -pub struct $service_name { +pub struct MdnsService { /// Main socket for listening. - socket: $udp_socket, + socket: Async, /// Socket for sending queries on the network. - query_socket: $udp_socket, + query_socket: Async, /// Interval for sending queries. - query_interval: Interval, + query_interval: Timer, /// Whether we send queries on the network at all. /// Note that we still need to have an interval for querying, as we need to wake up the socket - /// regularly to recover from errors. Otherwise we could simply use an `Option`. + /// regularly to recover from errors. Otherwise we could simply use an `Option`. silent: bool, /// Buffer used for receiving data from the main socket. /// RFC6762 discourages packets larger than the interface MTU, but allows sizes of up to 9000 @@ -133,55 +127,54 @@ pub struct $service_name { send_buffers: Vec>, /// Buffers pending to send on the query socket. query_send_buffers: Vec>, + /// Iface watch. + if_watch: IfWatcher, } -impl $service_name { +impl MdnsService { /// Starts a new mDNS service. - pub fn new() -> io::Result<$service_name> { - Self::new_inner(false) + pub async fn new() -> io::Result { + Self::new_inner(false).await } /// Same as `new`, but we don't automatically send queries on the network. - pub fn silent() -> io::Result<$service_name> { - Self::new_inner(true) + pub async fn silent() -> io::Result { + Self::new_inner(true).await } /// Starts a new mDNS service. - fn new_inner(silent: bool) -> io::Result<$service_name> { - let std_socket = { + async fn new_inner(silent: bool) -> io::Result { + let socket = { + let socket = Socket::new(Domain::ipv4(), Type::dgram(), Some(socket2::Protocol::udp()))?; + socket.set_reuse_address(true)?; #[cfg(unix)] - fn platform_specific(s: &net2::UdpBuilder) -> io::Result<()> { - net2::unix::UnixUdpBuilderExt::reuse_port(s, true)?; - Ok(()) - } - #[cfg(not(unix))] - fn platform_specific(_: &net2::UdpBuilder) -> io::Result<()> { Ok(()) } - let builder = net2::UdpBuilder::new_v4()?; - builder.reuse_address(true)?; - platform_specific(&builder)?; - builder.bind(("0.0.0.0", 5353))? + socket.set_reuse_port(true)?; + socket.bind(&SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), 5353).into())?; + let socket = socket.into_udp_socket(); + socket.set_multicast_loop_v4(true)?; + socket.set_multicast_ttl_v4(255)?; + Async::new(socket)? }; - let socket = $udp_socket_from_std(std_socket)?; // Given that we pass an IP address to bind, which does not need to be resolved, we can // use std::net::UdpSocket::bind, instead of its async counterpart from async-std. - let query_socket = $udp_socket_from_std( - std::net::UdpSocket::bind((Ipv4Addr::from([0u8, 0, 0, 0]), 0u16))?, - )?; + let query_socket = { + let socket = std::net::UdpSocket::bind((Ipv4Addr::UNSPECIFIED, 0))?; + Async::new(socket)? + }; + - socket.set_multicast_loop_v4(true)?; - socket.set_multicast_ttl_v4(255)?; - // TODO: correct interfaces? - socket.join_multicast_v4(From::from([224, 0, 0, 251]), Ipv4Addr::UNSPECIFIED)?; + let if_watch = if_watch::IfWatcher::new().await?; - Ok($service_name { + Ok(Self { socket, query_socket, - query_interval: Interval::new_at(Instant::now(), Duration::from_secs(20)), + query_interval: Timer::interval_at(Instant::now(), Duration::from_secs(20)), silent, recv_buffer: [0; 4096], send_buffers: Vec::new(), query_send_buffers: Vec::new(), + if_watch, }) } @@ -247,18 +240,8 @@ impl $service_name { } } - // Either (left) listen for incoming packets or (right) send query packets whenever the - // query interval fires. - let selected_output = match futures::future::select( - Box::pin(self.socket.recv_from(&mut self.recv_buffer)), - Box::pin(self.query_interval.next()), - ).await { - future::Either::Left((recved, _)) => Left(recved), - future::Either::Right(_) => Right(()), - }; - - match selected_output { - Left(left) => match left { + select! { + res = self.socket.recv_from(&mut self.recv_buffer).fuse() => match res { Ok((len, from)) => { match MdnsPacket::new_from_bytes(&self.recv_buffer[..len], from) { Some(packet) => return (self, packet), @@ -270,7 +253,7 @@ impl $service_name { // The query interval will wake up the task at some point so that we can try again. }, }, - Right(_) => { + _ = self.query_interval.next().fuse() => { // Ensure underlying task is woken up on the next interval tick. while let Some(_) = self.query_interval.next().now_or_never() {}; @@ -278,13 +261,42 @@ impl $service_name { let query = dns::build_query(); self.query_send_buffers.push(query.to_vec()); } + }, + event = self.if_watch.next().fuse() => { + let multicast = From::from([224, 0, 0, 251]); + let socket = self.socket.get_ref(); + match event { + Ok(IfEvent::Up(inet)) => { + if inet.addr().is_loopback() { + continue; + } + if let IpAddr::V4(addr) = inet.addr() { + log::trace!("joining multicast on iface {}", addr); + if let Err(err) = socket.join_multicast_v4(&multicast, &addr) { + log::error!("join multicast failed: {}", err); + } + } + } + Ok(IfEvent::Down(inet)) => { + if inet.addr().is_loopback() { + continue; + } + if let IpAddr::V4(addr) = inet.addr() { + log::trace!("leaving multicast on iface {}", addr); + if let Err(err) = socket.leave_multicast_v4(&multicast, &addr) { + log::error!("leave multicast failed: {}", err); + } + } + } + Err(err) => log::error!("if watch returned an error: {}", err), + } } }; } } } -impl fmt::Debug for $service_name { +impl fmt::Debug for MdnsService { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt.debug_struct("$service_name") .field("silent", &self.silent) @@ -292,17 +304,6 @@ impl fmt::Debug for $service_name { } } -}; -} - -#[cfg(feature = "async-std")] -codegen!("async-std", MdnsService, async_std::net::UdpSocket, (|socket| Ok::<_, std::io::Error>(async_std::net::UdpSocket::from(socket)))); - -// Note: Tokio's UdpSocket::from_std does not set the socket into non-blocking mode. -#[cfg(feature = "tokio")] -codegen!("tokio", TokioMdnsService, tokio::net::UdpSocket, (|socket: std::net::UdpSocket| { socket.set_nonblocking(true); tokio::net::UdpSocket::from_std(socket) })); - - /// A valid mDNS packet received by the service. #[derive(Debug)] pub enum MdnsPacket { @@ -595,7 +596,7 @@ mod tests { fn discover(peer_id: PeerId) { let fut = async { - let mut service = <$service_name>::new().unwrap(); + let mut service = <$service_name>::new().await.unwrap(); loop { let next = service.next().await; @@ -639,7 +640,7 @@ mod tests { .collect(); let fut = async { - let mut service = <$service_name>::new().unwrap(); + let mut service = <$service_name>::new().await.unwrap(); let mut sent_queries = vec![]; @@ -690,17 +691,15 @@ mod tests { } } - #[cfg(feature = "async-std")] testgen!( async_std, crate::service::MdnsService, (|fut| async_std::task::block_on::<_, ()>(fut)) ); - #[cfg(feature = "tokio")] testgen!( tokio, - crate::service::TokioMdnsService, + crate::service::MdnsService, (|fut| tokio::runtime::Runtime::new().unwrap().block_on::>(fut)) ); } diff --git a/src/lib.rs b/src/lib.rs index 8481a3221a20..514c352d8877 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -196,8 +196,8 @@ pub use libp2p_gossipsub as gossipsub; #[cfg_attr(docsrs, doc(cfg(feature = "mplex")))] #[doc(inline)] pub use libp2p_mplex as mplex; -#[cfg(any(feature = "mdns-async-std", feature = "mdns-tokio"))] -#[cfg_attr(docsrs, doc(cfg(any(feature = "mdns-async-std", feature = "mdns-tokio"))))] +#[cfg(feature = "mdns")] +#[cfg_attr(docsrs, doc(cfg(feature = "mdns")))] #[cfg(not(any(target_os = "emscripten", target_os = "wasi", target_os = "unknown")))] #[doc(inline)] pub use libp2p_mdns as mdns;