Remove `NotifyHandler::All` thus removing the requirement for events
send from a `NetworkBehaviour` to a `ProtocolsHandler` to be `Clone`. An
implementor of `NetworkBehaviour` can still notify all
`ProtocolHandler`s for a given peer by emitting one `NotifyHandler`
event per connection to that peer.
A user of libp2p-request-response is guaranteed to receive an additional event
after receiving a request via `RequestResponseEvent::Message`.
After receiving the request:
- If the user responds in time and the connection is still alive, the
user can expect a `ResponseSent`.
- If the user drops the response channel, the user can expect an
`InboundFailure::ResponseOmission`.
- If the user does not respond in time, the user can expect an
`InboundFailure::Timeout`.
Thus far the user did not receive an event when the connection to the
peer closes. With this commit:
- If the connection to the peer closes before the users calls
`send_response` or after the user calls `send_response` but before the
response can be send on the network, the user can expect an
`InboundFailure::ConnectionClosed`.
* Refine error reporting for inbound request handling.
At the moment one can neither get confirmation when a
response has been sent on the underlying transport, nor
is one aware of response omissions. The latter was
originally intended as a feature for support of
one-way protocols, which seems like a bad idea in
hindsight. The lack of notification for sent
responses may prohibit implementation of some
request-response protocols that need to ensure
a happens-before relation between sending a
response and a subsequent request, besides uses
for collecting statistics.
Even with these changes, there is no active notification
for failed inbound requests as a result of connections
unexpectedly closing, as is the case for outbound requests.
Instead, for pending inbound requests this scenario
can be identified if necessary by the absense of both
`InboundFailure` and `ResponseSent` events for a particular
previously received request. Interest in this situation is
not expected to be common and would otherwise require
explicitly tracking all inbound requests in the `RequestResponse`
behaviour, which would be a pity. `RequestResponse::send_response`
now also synchronously returns an error if the inbound upgrade
handling the request has been aborted, due to timeout or
closing of the connection, giving more options for graceful
error handling for inbound requests.
As an aside, the `Throttled` wrapper now no longer emits
inbound or outbound error events occurring in the context
of sending credit requests or responses. This is in addition
to not emitting `ResponseSent` events for ACK responses of
credit grants.
* Update protocols/request-response/src/lib.rs
Co-authored-by: Max Inden <mail@max-inden.de>
* Address some minor clippy warnings. (#1868)
* Track pending credit request IDs.
In order to avoid emitting events relating to credit grants or acks
on the public API. The public API should only emit events relating
to the actual requests and responses sent by client code.
* Small cleanup
* Cleanup
* Update versions and changelogs.
* Unreleased
Co-authored-by: Max Inden <mail@max-inden.de>
* 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 <pierre.krieger1708@gmail.com>
* Fix compilation.
* Update CHANGELOG
Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>
* feat: upgrade to multihash 0.13
`multihash` changes a lot internally, it is using stack allocation instead
of heap allocation. This leads to a few limitations in regards on how
`Multihash` can be used.
Therefore `PeerId` is now using a `Bytes` internally so that only minimal
changes are needed.
* Update versions and changelogs.
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
Co-authored-by: Roman S. Borschel <roman@parity.io>
* Streamline mplex and yamux configurations.
* For all configuration options that exist for both multiplexers
and have the same semantics, use the same names for the
configuration.
* Rename `Config` to `YamuxConfig` for consistentcy with
the majority of other protocols, e.g. `MplexConfig`, `PingConfig`,
`KademliaConfig`, etc.
* Completely hide `yamux` APIs within `libp2p-yamux`. This allows
to fully control the libp2p API and streamline it with other
muxer APIs, consciously choosing e.g. which configuration options
to make configurable in libp2p and which to fix to certain values.
It does also not necessarily prescribe new incompatible version bumps of
yamux for `libp2p-yamux`, as no `yamux` types are exposed. The cost
is some more duplication of configuration options in the API, as well
as the need to update `libp2p-yamux` if `yamux` introduces new
configuration options that `libp2p-yamux` wants to expose as well.
* Update CHANGELOGs.
In all cases, we pass the PeerId directly as the connection info.
The flexbility of doing something different here was originally
envisioned but turned out to be never needed.
For reference see: https://github.com/libp2p/rust-libp2p/issues/1798#issuecomment-714526056
Co-authored-by: Max Inden <mail@max-inden.de>
* [multistream-select] Fix panic with V1Lazy and add integration tests.
Fixes a panic when using the `V1Lazy` negotiation protocol,
a regression introduced in https://github.com/libp2p/rust-libp2p/pull/1484.
Thereby adds integration tests for a transport upgrade with both
`V1` and `V1Lazy` to the `multistream-select` crate to prevent
future regressions.
* Cleanup.
* Update changelog.
* Restore `RequestResponse::throttled`.
In contrast to the existing "throttled" approach this PR adds back-
pressure to the protocol without requiring pre-existing knowledge
of all nodes about their limits. It adds small, CBOR-encoded headers
to the actual payload data. Extra credit messages communicate back
to the sender how many more requests it is allowed to send.
* Remove some noise.
* Resend credit grant after connection closed.
Should an error in some lower layer cause a connection to be closed,
our previously sent credit grant may not have reached the remote peer.
Therefore, pessimistically, a credit grant is resent whenever a
connection is closed. The remote ignores duplicate grants.
* Remove inbound/outbound tracking per peer.
* Send ACK as response to duplicate credit grants.
* Simplify.
* Fix grammar.
* Incorporate review feedback.
- Remove `ResponseSent` which was a leftover from previous attemps
and issue a credit grant immediately in `send_response`.
- Only resend credit grants after a connection is closed if we are
still connected to this peer.
* Move codec/header.rs to throttled/codec.rs.
* More review suggestions.
* Generalise `ProtocolWrapper` and use shorter prefix.
* Update protocols/request-response/src/lib.rs
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
* Update protocols/request-response/src/throttled.rs
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
* Update protocols/request-response/src/throttled.rs
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
* Minor comment changes.
* Limit max. header size to 8KiB
* Always construct initial limit with 1.
Since honest senders always assume a send budget of 1 and wait for
credit afterwards, setting the default limit to a higher value
can only become effective after informing the peer about it which
means leaving `max_recv` at 1 and setting `next_max` to the desired
value.
* Use LRU cache to keep previous peer infos.
Peers may send all their requests, reconnect and send again all their
requests, starting from a fresh budget. The LRU cache keeps the peer
information around and reuses it when the peer reconnects, continuing
with the previous remaining limit.
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
* Restore `RequestResponse::throttled`.
In contrast to the existing "throttled" approach this PR adds back-
pressure to the protocol without requiring pre-existing knowledge
of all nodes about their limits. It adds small, CBOR-encoded headers
to the actual payload data. Extra credit messages communicate back
to the sender how many more requests it is allowed to send.
* Remove some noise.
* Resend credit grant after connection closed.
Should an error in some lower layer cause a connection to be closed,
our previously sent credit grant may not have reached the remote peer.
Therefore, pessimistically, a credit grant is resent whenever a
connection is closed. The remote ignores duplicate grants.
* Remove inbound/outbound tracking per peer.
* Send ACK as response to duplicate credit grants.
* Simplify.
* Fix grammar.
* Incorporate review feedback.
- Remove `ResponseSent` which was a leftover from previous attemps
and issue a credit grant immediately in `send_response`.
- Only resend credit grants after a connection is closed if we are
still connected to this peer.
* Move codec/header.rs to throttled/codec.rs.
* More review suggestions.
* Generalise `ProtocolWrapper` and use shorter prefix.
* Update protocols/request-response/src/lib.rs
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
* Update protocols/request-response/src/throttled.rs
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
* Update protocols/request-response/src/throttled.rs
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
* Minor comment changes.
* Limit max. header size to 8KiB
* Always construct initial limit with 1.
Since honest senders always assume a send budget of 1 and wait for
credit afterwards, setting the default limit to a higher value
can only become effective after informing the peer about it which
means leaving `max_recv` at 1 and setting `next_max` to the desired
value.
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
The codec impl did not check that it actually read any bytes in
`read_request` and `read_response`. The used `read_one` function
does not error on EOF either, so instead of signalling connection
loss the codec could produce empty `Ping` or `Pong` messages.
* Use a single exchange instead of two one_shots.
* Add `Throttled` to libp2p-request-response.
Wraps the existing `RequestResponse` behaviour and applies strict limits
to the number of inbound and outbound requests per peer.
The wrapper is opt-in and if not used, the protocol behaviour of
`RequestResponse` does not change. This PR also does not introduce
an extra protocol, hence the limits applied need to be known a priori
for all nodes which is not always possible or desirable. As mentioned
in #1687 I think that we should eventually augment the protocol with
metadata which allows a more dynamic exchange of requests and responses.
This PR also replaces the two oneshot channels with a single one from the
scambio crate which saves one allocation per request/response. If not
desirable because the crate has seen less testing the first commit could
be reverted.
* Fix rustdoc error.
* Remove some leftovers from development.
* Add docs to `NetworBehaviourAction::{map_in,map_out}`.
* Apply suggestions from code review
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
* Add `ping_protocol_throttled` test.
* Add another test.
* Revert "Use a single exchange instead of two one_shots."
This reverts commit e34e1297d411298f6c69e238aa6c96e0b795d989.
# Conflicts:
# protocols/request-response/Cargo.toml
# protocols/request-response/src/handler/protocol.rs
* Update CHANGELOG.
* Update CHANGELOG.
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
* Emit events for active connection close and fix `disconnect()`.
The `Network` does currently not emit events for actively
closed connections, e.g. via `EstablishedConnection::close`
or `ConnectedPeer::disconnect()`. As a result, when actively
closing connections, there will be `ConnectionEstablished`
events emitted without eventually a matching `ConnectionClosed`
event. This seems undesirable and has the consequence that
the `Swarm::ban_peer_id` feature in `libp2p-swarm` does not
result in appropriate calls to `NetworkBehaviour::inject_connection_closed`
and `NetworkBehaviour::inject_disconnected`. Furthermore,
the `disconnect()` functionality in `libp2p-core` is currently
broken as it leaves the `Pool` in an inconsistent state.
This commit does the following:
1. When connection background tasks are dropped
(i.e. removed from the `Manager`), they
always terminate immediately, without attempting
an orderly close of the connection.
2. An orderly close is sent to the background task
of a connection as a regular command. The
background task emits a `Closed` event
before terminating.
3. `Pool::disconnect()` removes all connection
tasks for the affected peer from the `Manager`,
i.e. without an orderly close, thereby also
fixing the discovered state inconsistency
due to not removing the corresponding entries
in the `Pool` itself after removing them from
the `Manager`.
4. A new test is added to `libp2p-swarm` that
exercises the ban/unban functionality and
places assertions on the number and order
of calls to the `NetworkBehaviour`. In that
context some new testing utilities have
been added to `libp2p-swarm`.
This addresses https://github.com/libp2p/rust-libp2p/issues/1584.
* Update swarm/src/lib.rs
Co-authored-by: Toralf Wittner <tw@dtex.org>
* Incorporate some review feedback.
* Adapt to changes in master.
* More verbose panic messages.
* Simplify
There is no need for a `StartClose` future.
* Fix doc links.
* Further small cleanup.
* Update CHANGELOGs and versions.
Co-authored-by: Toralf Wittner <tw@dtex.org>
* Close substream after writing request/response.
* Update protocols/request-response/src/handler/protocol.rs
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
* Add the libp2p-request-response protocol.
This crate provides a generic implementation for request/response
protocols, whereby each request is sent on a new substream.
* Fix OneShotHandler usage in floodsub.
* Custom ProtocolsHandler and multiple protocols.
1. Implement a custom ProtocolsHandler instead of using
the OneShotHandler for better control and error handling.
In particular, all request/response sending/receiving is
kept in the substreams upgrades and thus the background
task of a connection.
2. Support multiple protocols (usually protocol versions)
with a single `RequestResponse` instance, with
configurable inbound/outbound support.
* Small doc clarification.
* Remove unnecessary Sync bounds.
* Remove redundant Clone constraint.
* Update protocols/request-response/Cargo.toml
Co-authored-by: Toralf Wittner <tw@dtex.org>
* Update dev-dependencies.
* Update Cargo.tomls.
* Add changelog.
* Remove Sync bound from RequestResponseCodec::Protocol.
Apparently the compiler just needs some help with the scope
of borrows, which is unfortunate.
* Try async-trait.
* Allow checking whether a ResponseChannel is still open.
Also expand the commentary on `send_response` to indicate that
responses may be discard if they come in too late.
* Add `RequestResponse::is_pending`.
As an analogue of `ResponseChannel::is_open` for outbound requests.
* Revert now unnecessary changes to the OneShotHandler.
Since `libp2p-request-response` is no longer using it.
* Update CHANGELOG for libp2p-swarm.
Co-authored-by: Toralf Wittner <tw@dtex.org>
