NodeConnectionManager manages remote initiated connections

[tegefaulkes]

Specification

With the creation and integration of @matrixai/js-quic, the concept of a connection is now generic to forward and reverse initiated connections. This means, no matter who initiated the connection, streams can be created in either direction.

Previously the NodeConnectionManager only managed forward initiated connections. Now it should be able to handle reverse connections and add them to the connection map. This is essential for re-using connections efficiently.

The NodeConnection needs to be updated to be able to wrap QUICConnections coming out of the QUICServer. This will be a new async creation method. The cleanup logic will need to switch based on if it was a forward connection with a QUICClient or a reverse connection.

The QUICServer needs to be moved into and managed by the NodeConnectionManager. It's lifecycle will be fully managed. I don't know if we want to be able to DI it.

Reverse connections coming out of the QUICServer needs to be handled, wrapped in a NodeConnection and added into the connection map. There may be some complexities with locking here, that still needs to be prototyped.

All reverse stream events coming out of the QUICConnectons need to be handled by the RPCServer via a handleStream callback.

Additional context

• Related js-quic integration and Agent migration #525

Tasks

• 1. Update NodeConnection to wrap reverse created QUICStreams and handle clean up in all cases. QUICClient can be optional and needs to be handled.
• 2. QUICServer needs to be moved and managed by NodeConnectionManager
• 3. Reverse connections coming from the QUICServer needs to be handled and put into the connection map.
• 4. Reverse streams from all QUICConnections needs to be handled by a RPCServer via a handleStream callback.
• 5. Tests need to be expanded and updated to handle new behaviour

[tegefaulkes]

So the main changes that need to be made are the following

1. NodeConnection needs to be more generic and use the QUICConnection directly instead of the QUICClient but it still needs to handle the QUICClient life-cycle if it exists.
2. NodeConnection needs two ways of being created, either via starting a connection or wrapping an existing QUICConnection from the server event.
3. NodeConnectionManager needs to handle connection events from the QUICServer, wrap the new connection into a NodeConnection and add it to the connection map.

On the first point, we need to make NodeConnection more generic. So all logic and creation must be done with the context of the QUICConnection. But if the NodeConnectionis creating theQUICConnectionthen theQUICClientcould exist and needs to be handled. It's possible we can just re-create the logic of theQUICClient for establishing the connection but maybe just handling the client is easier.

On the 2nd point, the NodeConnection needs to methods for creating it. The first handles the creation of the QUICConnection and establishes the connection. The 2nd constructs the NodeConnection from an existing QUICConnection. Extra logic needs to be added to handle if the QUICConnection was created via a client or server since there are extra life-cycle considerations.

The 3rd point may be the trickiest. We need to handle the incoming connections and add them to the object map. This means handling a QUICServer connection event, Wrapping the connection into a NodeConnection and adding it into the object map while handling the connection map locking. The QUICServer and RPCServer should be moved into and managed by the NodeConnectionManager now.

The main point of fuzziness with the design right now is how we handle the connection map locking. It would be possible for a race condition where two nodes attempt a connection to each other at the same time. Since there should only be one connection how do we decide which one to use? We will only know about an incoming connection after it has established and verified, so that should take precedence to any running connection attempt for that node. Extra care needs to be taken when handling this.

[tegefaulkes]

I think this can and should be done after the agent migration work is done. Possible as a new PR, just so we have a clean point to work from.

[CMCDragonkai]

Wouldn't integration of js-quic lead to this in one go since agent RPC migration is already done?

[tegefaulkes]

It could be done while integrating js-quic, but I think it will be better to handle this in a separate stage of changes. Remember that we want to avoid expanding the scope of issues and PRs if we can avoid it.

[tegefaulkes]

Based on a discussion. The quicServer will be internal to and it's lifecycle managed by the NodeConnectionManager. But the rpcServer that handles the streams must be kept outside of the NodeConnectionManager The streams will be handled by the usual method of a stream handler callback.

[tegefaulkes]

Order of work goes as follows.

1. Update NodeConnection to include a creation method for wrapping an existing QUICConnection. This will need to wrap the connection directly, but also track the QUICClient to clean that up if needed.
2. Update NodeConnection tests to test new changes.
3. Move QUICServer into and have managed by NodeConnectionManager. RPCServer` will still be separate but handle streams via the usual method.
4. Have reverse connections he handled by the NodeConnectionManager and insert them into the connection map.
5. Have all connections reverse stream creation events be handled by the RPCServer via a handleStream callback.
6. Update and expand tests with new behaviour.

[tegefaulkes]

Almost done here. Just need to do some clean up and fix tests. Maybe take a look at locking and connection creation and see if I can tidy it up with monitors.

While working on this I was able to combine the client and server side QUICConfigs. Is this something we'd want to keep separate? At the end of the day, they're all connections in the connection map. Not sure we want them to act different depending if they're forward or reverse created.

[tegefaulkes]

This is done now. Looking into either removing UWS or fixing up vaults cloning/pulling next.

[CMCDragonkai]

Almost done here. Just need to do some clean up and fix tests. Maybe take a look at locking and connection creation and see if I can tidy it up with monitors.

While working on this I was able to combine the client and server side QUICConfigs. Is this something we'd want to keep separate? At the end of the day, they're all connections in the connection map. Not sure we want them to act different depending if they're forward or reverse created.

Not in the case of PK. But in other scenarios... it may be necessary. It also depends, do both actually use the same options?

[CMCDragonkai]

If this is done, are you working against a PR that can close this issue or?

[tegefaulkes]

I'm working out of #535