refactor(relay): remove KeepAlive::Until

[leonzchang]

Description

KeepAlive::Until is being deprecated. We emulate the functionality within the ConnectionHandler to ensure relayed connections don't get immediately closed when a client establishes one.

Related: #3844.
Related: #4656.

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

[leonzchang]

Since this change doesn't break anything, I guess we can just merge it?

[thomaseizinger]

Since this change doesn't break anything, I guess we can just merge it?

Possibly! I haven't looked closely at the code yet. Is there an opportunity to also refactor towards a preload_handler solution? Ideally, we shouldn't depend on an idle connection timeout being set.

[leonzchang]

Since this change doesn't break anything, I guess we can just merge it?

Possibly! I haven't looked closely at the code yet. Is there an opportunity to also refactor towards a preload_handler solution? Ideally, we shouldn't depend on an idle connection timeout being set.

Since relay::Behaviour doesn't do anything on connection established, I think this crate doesn't need to refactor?

I will check on the other PRs.

[thomaseizinger]

Since this change doesn't break anything, I guess we can just merge it?

Possibly! I haven't looked closely at the code yet. Is there an opportunity to also refactor towards a preload_handler solution? Ideally, we shouldn't depend on an idle connection timeout being set.

Since relay::Behaviour doesn't do anything on connection established, I think this crate doesn't need to refactor?

Well, I just thought about this a bit more in detail and it might not be that easy unfortunately. It is correct that a relay server never does anything on its own on an inbound connection. Those are opened by clients to make reservations with we have to keep the connection alive for this to work properly.

I think this might be a valid case to have a functionality like KeepAlive::Until but we'd probably have to emulate it ourselves.

• For outbound connections, I think we can always return KeepAlive::No.
• For inbound connections, we should keep them alive for a bit longer.

I pushed a follow-up commit that implements that. Curious to hear what @mxinden thinks.

[thomaseizinger]

For outbound connections, I think we can always return KeepAlive::No.

I am wondering why we even construct a handler for outbound connections. @mxinden would you agree that we don't need to support the relay protocol on outbound connections? Would it be fair to say that this is a left-over from when we didn't distinguish between inbound and outbound connections as explicitly?

[mxinden]

For outbound connections, I think we can always return KeepAlive::No.

I am wondering why we even construct a handler for outbound connections. @mxinden would you agree that we don't need to support the relay protocol on outbound connections? Would it be fair to say that this is a left-over from when we didn't distinguish between inbound and outbound connections as explicitly?

I see a couple of scenarios, none of them particularly common, for supporting circuit relay v2 streams on an outbound connection as a relay server:

• A client that is public, but that uses the relay server to connect to a third node that it doesn't have any transport protocols in common with. E.g. the client speaks TCP, the third node speaks QUIC, the relay speaks both.
• The server has an existing outbound hole punched connection to the client, for whatever reason. Instead of creating a new connection, the client re-uses the existing connection.

[mxinden]

One comment. Otherwise this looks good to me.