6 Commits

Author SHA1 Message Date
Benno
30024f38d4
fix: clippy useless_conversion and useless_vec
Pull-Request: #4190.
2023-07-16 03:31:37 +00:00
Max Inden
76cb76ca7d
fix(multistream-select): don't wait for negotiation in poll_close
With `Version::V1Lazy` and negotiation of a single protocol, a stream initiator optimistically
sends application data right after proposing its protocol. More specifically an application can
write data via `AsyncWrite::poll_write` even though the remote has not yet confirmed the stream protocol.

This saves one round-trip.

``` mermaid
sequenceDiagram
A->>B: "/multistream/1.0.0"
A->>B: "/perf/1.0.0"
A->>B: <some-perf-protocol-data>
B->>A: "/multistream/1.0.0"
B->>A: "/perf/1.0.0"
B->>A: <some-perf-protocol-data>
```

When considering stream closing, i.e. `AsyncWrite::poll_close`, and using stream closing as an
operation in ones protocol, e.g. using stream closing to signal the end of a request, this becomes tricky.

The behavior without this commit was as following:

``` mermaid
sequenceDiagram
A->>B: "/multistream/1.0.0"
A->>B: "/perf/1.0.0"
A->>B: <some-perf-protocol-data>
Note left of A: Call `AsyncWrite::poll_close` which first waits for the<br/>optimistic multistream-select negotiation to finish, before closing the stream,<br/> i.e. setting the FIN bit.
B->>A: "/multistream/1.0.0"
B->>A: "/perf/1.0.0"
Note right of B: Waiting for A to close the stream (i.e. set the `FIN` bit)<br/>before sending the response.
A->>B: FIN
B->>A: <some-perf-protocol-data>
```

The above takes 2 round trips:

1. Send the optimistic multistream-select protocol proposals as well as the initiator protocol
payload and waits for the confirmation of the protocols.
2. Close the stream, i.e. sends the `FIN` bit and waits for the responder protocol payload.

This commit proposes that the stream initiator should not wait for the multistream-select protocol
confirmation when closing the stream, but close the stream within the first round-trip.

``` mermaid
sequenceDiagram
A->>B: "/multistream/1.0.0"
A->>B: "/perf/1.0.0"
A->>B: <some-perf-protocol-data>
A->>B: FIN
B->>A: "/multistream/1.0.0"
B->>A: "/perf/1.0.0"
B->>A: <some-perf-protocol-data>
```

This takes 1 round-trip.

The downside of this commit is, that the stream initiator will no longer notice a negotiation error
when closing the stream. They will only notice it when reading from the stream. E.g. say that B does
not support "/perf/1.0.0", A will only notice on `AsyncRead::poll_read`, not on
`AsyncWrite::poll_close`. This is problematic for protocols where A only sends data, but never
receives data, i.e. never calls `AsyncRead::poll_read`. Though one can argue that such protocol is
flawed in the first place. With a response-less protocol, as even if negotiation succceeds, A
doesn't know whether B received the protocol payload.

Pull-Request: #4019.
2023-06-06 21:00:20 +00:00
Max Inden
fb4f6dfe31
chore(multistream-select): use futures_ringbuf
Pull-Request: #4032.
2023-06-06 04:29:41 +00:00
Thomas Eizinger
c93f753018
feat: replace ProtocolName with AsRef<str>
Previously, a protocol could be any sequence of bytes as long as it started with `/`. Now, we directly parse a protocol as `String` which enforces it to be valid UTF8.

To notify users of this change, we delete the `ProtocolName` trait. The new requirement is that users need to provide a type that implements `AsRef<str>`.

We also add a `StreamProtocol` newtype in `libp2p-swarm` which provides an easy way for users to ensure their protocol strings are compliant. The newtype enforces that protocol strings start with `/`. `StreamProtocol` also implements `AsRef<str>`, meaning users can directly use it in their upgrades.

`multistream-select` by itself only changes marginally with this patch. The only thing we enforce in the type-system is that protocols must implement `AsRef<str>`.

Resolves: #2831.

Pull-Request: #3746.
2023-05-04 04:47:11 +00:00
Hannes
d79c93abdb
chore: Implement latest clippy warnings (#3220)
As I do frequently, I corrected for the latest clippy warnings. This will make sure the CI won't complain in the future. We could automate this btw and maybe run the nightly version of clippy.
2022-12-14 15:45:04 +00:00
Friedel Ziegelmayer
c71115d055
misc/multistream-select/: Remove parallel dialing optimization (#2934)
This is to avoid the usage of the now optional `ls` command, and
stay compatible with go-multistream.

Closes #2925
2022-09-29 14:36:11 +01:00