fix(hole-punch): don't explicitly close RESERVE stream

[thomaseizinger]

Description

Making a reservation on a relay involves a request-response protocol. First we send the RESERVE message and then we receive the relay's response. After we've received the response, we have all the information we need and the reservation is valid.

It appears that on QUIC, closing the stream can fail if the other party also closed the stream already. This can result in the entire operation failing at the very end even though everything else has succeeded.

There is no reason to explicitly close the stream as the remote is not going to read from it beyond the first message anyway. The fact that we receive a response means the remote read our request.

This should resolve some of the flakiness that we've been seeing with the hole-punch interop tests.

Notes & open questions

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• A changelog entry has been made in the appropriate crates

[thomaseizinger]

@mxinden Please feel free to weigh in with your opinion on this. Based on the above reasoning, I don't think this is particularly controversial, hence I am moving forward with this so that I can update the test-plans repository which in turn should resolve the flakiness we've been seeing in the other PRs.

[thomaseizinger]

@mxinden I don't think it matters whether we drop the stream or close it properly. If we wanted to close it properly, we could do so directly after sending the message perhaps?

[mxinden]

It appears that on QUIC, closing the stream can fail if the other party also closed the stream already.

I thought we fixed this in the past when still using quinn-proto and I thought moving to quinn directly did so as well. #3164

Instead of a fix at the libp2p-relay level, I think this requires a fix at the libp2p-quic layer.

If we wanted to close it properly, we could do so directly after sending the message perhaps?

In my eyes that would be the right thing to do, i.e. close right after writing the reservation request, before reading the reservation response.

[thomaseizinger]

It appears that on QUIC, closing the stream can fail if the other party also closed the stream already.

I thought we fixed this in the past when still using quinn-proto and I thought moving to quinn directly did so as well. #3164

Instead of a fix at the libp2p-relay level, I think this requires a fix at the libp2p-quic layer.

Agreed that this needs to be the permanent fix. I'll try to reproduce it using a test in quinn.

If we wanted to close it properly, we could do so directly after sending the message perhaps?

In my eyes that would be the right thing to do, i.e. close right after writing the reservation request, before reading the reservation response.

We can reintroduce that once it doesn't cause flaky tests. It doesn't make a difference for the protocol itself though :)

[thomaseizinger]

I opened a PR: quinn-rs/quinn#1699.