According to
https://github.com/rust-lang/project-error-handling/issues/44#issuecomment-858151860,
we should not be printing the inner error AND returning it as source. Libraries
like `anyhow` will traverse the chain of `source` errors and build up a composed
error message. Printing it and returning the same error from `source` results in
duplicate error messages in that case.
A `ProtocolsHandler`, now `ConnectionHandler`, handels a connection, not
a protocol. Thus the name `CONNECTIONHandler` is more appropriate.
Next to the rename of `ProtocolsHandler` this commit renames the `mod
protocols_handler` to `mod handler`. Finally all combinators (e.g.
`ProtocolsHandlerSelect`) are renamed appropriately.
Remove `SignedEnvelope::payload` in favor of
`SignedEnvelope::payload_and_signing_key`. The caller is expected to check that
the returned signing key makes sense in the payload's context.
Previously one would wrap a `ProtocolsHandler` into a
`NodeHandlerWrapper` as early as possible, even though the functionality
of `NodeHandlerWrapper` is only needed within `mod connection`.
This commit makes `NodeHandlerWrapper` an implementation detail of `mod
connection`, thus neither `mod protocols_handler`, `mod pool` nor the
root level (`libp2p-swarm`) need to bother about the abstraction.
In addition to the above, this commit:
- Renames `NodeHandlerWrapper` to `HandlerWrapper`. The word `Node` is
outdated.
- Removes `NodeHandlerWrapperBuilder`. With this simplification it is no
longer needed.
- Folds `NodeHandlerWrapperError` into `ConnectionError`. No need for
upper layers to be aware of the fact that `ProtocolHandler`s are
wrapped.
The `ConnectionHandler` trait is not exposed to users. The only
implementor of `ConnectionHandler` is `NodeHandlerWrapper`. Thus
`ConnectionHandler` is a superfluous abstraction. This commit removes
`ConnectionHandler`.
Next to this large change, this commit removes the `Tmuxer` trait
parameter. `Swarm` enforces dynamic dispatching via `StreamMuxerBox`
anyways, thus the trait parameter is useless.
As a follow up to this commit one could rename `ProtocolsHandler` to
`ConnectionHandler` and `NodeHandlerWrapper` to
`ConnectionHandlerWrapper` or just `Wrapper`.
Disconnect pending connections with `Swarm::disconnect` and eport aborted
connections via `SwarmEvent::OutgoingConnectionError`.
Co-authored-by: Jack Maloney <git@jmmaloney4.xyz>
Co-authored-by: Marco Munizaga <git@marcopolo.io>
This commit removes the `Network` abstraction, thus managing `Listeners`
and the connection `Pool` in `Swarm` directly. This is done under the
assumption that noone uses the `Network` abstraction directly, but
instead everyone always uses it through `Swarm`. Both `Listeners` and
`Pool` are moved from `libp2p-core` into `libp2p-swarm`. Given that they
are no longer exposed via `Network`, they can be treated as an
implementation detail of `libp2p-swarm` and `Swarm`.
This change does not include any behavioural changes.
This change has the followin benefits:
- Removal of `NetworkEvent`, which was mostly an isomorphism of
`SwarmEvent`.
- Removal of the never-directly-used `Network` abstraction.
- Removal of now obsolete verbose `Peer` (`core/src/network/peer.rs`)
construct.
- Removal of `libp2p-core` `DialOpts`, which is a direct mapping of
`libp2p-swarm` `DialOpts`.
- Allowing breaking changes to the connection handling and `Swarm` API
interface without a breaking change in `libp2p-core` and thus a
without a breaking change in `/transport` protocols.
This change enables the following potential future changes:
- Removal of `NodeHandler` and `ConnectionHandler`. Thus allowing to
rename `ProtocolsHandler` into `ConnectionHandler`.
- Moving `NetworkBehaviour` and `ProtocolsHandler` into `libp2p-core`,
having `libp2p-xxx` protocol crates only depend on `libp2p-core` and
thus allowing general breaking changes to `Swarm` without breaking all
`libp2p-xxx` crates.
There are 3 ways to close a connection:
1. `ProtocolsHandlerEvent::Close`
2. `KeepAlive::No` via `ProtocolsHandler::connection_keep_alive`
3. `NetworkBehaviourAction::CloseConnection`
This commit references (2) as an alternative to (1) in the documentation
of (1).
Within `Provider::new_stream` we wait for the socket to become writable
(`stream.writable`), before returning it as a stream. In other words, we
are waiting for the socket to connect before returning it as a new TCP
connection. Waiting to connect before returning it as a new TCP
connection allows us to catch TCP connection establishment errors early.
While `stream.writable` drives the process of connecting, it does not
surface potential connection errors themselves. These need to be
explicitly collected via `TcpSocket::take_error`. If not explicitly
collected, they will surface on future operations on the socket.
For now this commit explicitly calls `TcpSocket::take_error` when using
`async-io` only. `tokio` introduced the method (`take_error`) in
https://github.com/tokio-rs/tokio/pull/4364 though later reverted it in
https://github.com/tokio-rs/tokio/pull/4392. Once re-reverted, the same
patch can be applied when using `libp2p-tcp` with tokio.
---
One example on how this bug surfaces today:
A `/dnsaddr/xxx` `Multiaddr` can potentially resolve to multiple IP
addresses, e.g. to the IPv4 and the IPv6 addresses of a node.
`libp2p-dns` tries dialing each of them in sequence using `libp2p-tcp`,
returning the first that `libp2p-tcp` reports as successful.
Say that the local node tries the IPv6 address first. In the scenario
where the local node's networking stack does not support IPv6, e.g. has
no IPv6 route, the connection attempt to the resolved IPv6 address of
the remote node fails. Given that `libp2p-tcp` does not call
`TcpSocket::take_error`, it would falsly report the TCP connection
attempt as successful. `libp2p-dns` would receive the "successful" TCP
connection for the IPv6 address from `libp2p-tcp` and would not attempt
to dial the IPv4 address, even though it supports IPv4, and instead
bubble up the "successful" IPv6 TCP connection. Only later, when writing
or reading from the "successful" IPv6 TCP connection, would the IPv6
error surface.
Co-authored-by: Oliver Wangler <oliver@wngr.de>
When a peer disconnects, reservations associated with that peer are removed from
the set of reservations of the peer. In case the set of reservations for the
peer is now empty, remove the entire peer.
Same when a reservation times out.
Previously, the negotiated PeerId was included in the swarm event and
inject_dial_failure’s arguments while the expected one was absent. This
patch adds the negotiated PeerId to the DialError and includes the expected
one in the notifications.
Co-authored-by: Roland Kuhn <rk@rkuhn.info>
