[Tracking issue]: Uniform substream behaviour and interface

[thomaseizinger]

This a tracking issue for all sorts of efforts that concern a uniform behaviour of libp2p streams, regardless of the underlying transport or muxer.

Requirements

• Streams should allow for closing the read and write side separately
• Streams should reset on drop, signalling an unexpected shutdown. This is yet to be defined properly in the specs: Carify usage and behaviour of stream resets specs#466

Done criteria

• We have a passing test for each substream implementation¹ for each requirement.

Action items

1. Implement a first version of a test harness that covers as much possible: muxers: Add test harness for StreamMuxer implementations #2952. This first version will only assert the subset of requirements that are passing without further modification of the muxer / transport implementations.
2. Investigate the root cause for muxers not passing certain requirements:
   1. yamux cannot read from a substream that has not been written to.
   2. yamux does not relay stream resets to the AsyncRead and AsyncWrite implementation.
   3. mplex does not relay stream resets to the AsyncRead and AsyncWrite implementation.
   4. ... to be extended as we write more tests
3. Add tests to the harness for observing stream resets in the AsyncRead and AsyncWrite implementations.
4. Extend test harness to test QUIC substreams
5. Extend test harness to test WebRTC substreams
6. Add an interface for closing the read-half of a substream.
7. Add tests to the harness for closing the read-half of a substeam.

Open work

• muxers: Add test harness for StreamMuxer implementations #2952
• Refactor Connection to a synchronous state machine rust-yamux#142: This will help with fixing the yamux issues because it makes the code much easier to reason about.
• Carify usage and behaviour of stream resets specs#466: Before we can implement the stream-reset tests, we need to agree on the expected behaviour.

Footnotes

1. mplex is considered deprecated and will not be fixed. Users should upgrade to yamux or another transport if they need a feature that is not supported. ↩

[mxinden]

Thanks for the tracking issue! I added it to our project board. I still need to prioritize it. Input welcome.

Overall looks good to me. Just one comment:

Streams should allow for closing the read and write side separately

I am not aware of a protocol that needs to close the read side. I suggest we delay implementing that half of the feature until there is need for it.

[thomaseizinger]

Streams should allow for closing the read and write side separately

I am not aware of a protocol that needs to close the read side. I suggest we delay implementing that half of the feature until there is need for it.

"That half". Pun intended? 😊

Makes sense! I moved it to the bottom of the action list. If we are unaware of a protocol that uses it, perhaps we should strip it from the specification too?

[mxinden]

f we are unaware of a protocol that uses it, perhaps we should strip it from the specification too?

Stripping it off the specification would prevent us from introducing it in the future, right?

[thomaseizinger]

f we are unaware of a protocol that uses it, perhaps we should strip it from the specification too?

Stripping it off the specification would prevent us from introducing it in the future, right?

Maybe, I haven't thought about it much. It seems odd to have every implementation be non-compliant 🤷‍♂️

[thomaseizinger]

Closing as mostly completed.