relay: refactor stream handling away from {In,Out}boundUpgrade

[thomaseizinger]

The libp2p-relay protocol currently makes heavy use of the InboundUpgrade and OutboundUpgrade traits. This creates a lot of complexity because we actually cannot "complete" those requests within the upgrade. Instead, we have to read a message and pass the message alongside with the stream an as an event to the ConnectionHandler.

Long-term, we want completely remove these upgrade traits for ConnectionHandlers, see #2863.

Before we can do this, we need to migrate all of our own protocols away from using these traits. This issue tracks doing that for the relay protocol.

The actual steps are:

• Instead of implementing our InboundUpgrade and OutboundUpgrade on our own types, we use ReadyUpgrade.
• ReadyUpgrade directly hands the stream to the ConnectionHandler
• In the ConnectionHandler, we can now either define a state machine ourselves or box up an async fn and store it inside a FuturesUnordered that gets polled as part of ConnectionHandler::poll
• What was previously happening inside InboundUpgrade::inbound_upgrade would now happen in the async fn that gets stored inside a FuturesUnordered

One thing that we need to take into account is that the futures must be accompanied with a timeout, i.e. we cannot just read from the stream forever.

[dgarus]

@thomaseizinger
Hello, Thomas!
I'd like to do this refactoring.

[thomaseizinger]

@thomaseizinger Hello, Thomas! I'd like to do this refactoring.

Thank you! Feel free to ping me with any questions and/or open a draft PR at any time.

[dgarus]

https://github.com/libp2p/specs/blob/master/relay/circuit-v2.md#stop-protocol

In order to terminate a relayed connection, the relay opens a stream using an existing connection to the target peer.

Is this a typo?

[thomaseizinger]

https://github.com/libp2p/specs/blob/master/relay/circuit-v2.md#stop-protocol

In order to terminate a relayed connection, the relay opens a stream using an existing connection to the target peer.

Is this a typo?

I think terminate in this context means the same as "terminal" i.e. the end-station of something.

First we have a reservation from A to R which can be considered an "open" circuit. Once R opens a stream to B, the circuit is closed (A and B can talk to each other) and thus, the end is reached aka terminated.

[dgarus]

https://github.com/libp2p/specs/blob/master/relay/circuit-v2.md#stop-protocol

In order to terminate a relayed connection, the relay opens a stream using an existing connection to the target peer.

Is this a typo?

I think terminate in this context means the same as "terminal" i.e. the end-station of something.

First we have a reservation from A to R which can be considered an "open" circuit. Once R opens a stream to B, the circuit is closed (A and B can talk to each other) and thus, the end is reached aka terminated.

Ok, I got it. Thanks!

[mxinden]

Threw me of in the past as well @dgarus. In case you can think of a better wording, please propose a pull request against libp2p/specs.

[dgarus]

@thomaseizinger
Hello, Thomas!
How to use ReadyUpgrade in the case when Upgrade has a state?
For example,

pub struct Upgrade {
    pub src_peer_id: PeerId,
    ...
}

src_peer_id comes from In::NegotiateOutboundConnect and we have to remember it.

[thomaseizinger]

What you do is remember the state locally in a VecDeque and request a new stream. Once you receive a stream, you pop_front the state and you can combine your state with the new stream. See https://github.com/libp2p/rust-libp2p/pull/3763/files for example.

[thomaseizinger]

What you do is remember the state locally in a VecDeque and request a new stream. Once you receive a stream, you pop_front the state and you can combine your state with the new stream. See #3763 (files) for example.

The reason this works is because streams are "interchangable", i.e. there is no identity tied to a requested stream and there also are no guarantees as to how data arrives at the remote when it is sent over multiple streams.

[dgarus]

@thomaseizinger
Hi, Thomas!
I'm on vacation, a lot of sunshine and wine don't allow me to make progress as fast as I would want.
So, I'm going to continue working on this issue in 2 weeks.

[thomaseizinger]

I'm on vacation, a lot of sunshine and wine don't allow me to make progress as fast as I would want.
So, I'm going to continue working on this issue in 2 weeks.

No pressure at all! Your dedication is much appreciated :)

I'll be on vacation too in the following week. Cheers 🍷

[thomaseizinger]

@dgarus I hope you had a nice vacation! I am curious whether you've already got some work locally that you can put up as a draft PR?

[dgarus]

@dgarus I hope you had a nice vacation! I am curious whether you've already got some work locally that you can put up as a draft PR?

@thomaseizinger
Hello! Thanks!
I finished with stop_protocol and currently I'm working on hop_protocol.