-
-
Save vasco-santos/bc89fe6e0acf8186f1137576135fbd49 to your computer and use it in GitHub Desktop.
const STATUS = { | |
OPEN: 0, | |
CLOSED: 1, | |
CLOSING: 2, | |
} | |
const ROLE = { | |
INITIATOR: 0, | |
RESPONDER: 1 | |
} | |
class Connection { | |
constructor (peerInfo, remoteMa, isInitiator) { | |
/** | |
* Connection identifier | |
*/ | |
this.id = (~~(Math.random() * 1e9)).toString(36) + Date.now() | |
/** | |
* Remote peer info | |
*/ | |
this.peerInfo = peerInfo | |
/** | |
* Status of the connection | |
*/ | |
this.status = STATUS.OPEN | |
/** | |
* Endpoints multiaddrs | |
*/ | |
this.endpoints = { | |
local: undefined, | |
remote: remoteMa | |
} | |
/** | |
* Connection timeline | |
*/ | |
this.timeline = { | |
openTs: Date.now(), | |
closeTs: undefined | |
} | |
/** | |
* Role in the connection, initiator or responder | |
*/ | |
this.role = isInitiator ? ROLE.INITIATOR : ROLE.RESPONDER | |
/** | |
* The multiplexer being used | |
*/ | |
this.multiplexer = undefined | |
/** | |
* The encryption method being used | |
*/ | |
this.encryption = undefined | |
/** | |
* Connection streams | |
*/ | |
this.streams = [] | |
/** | |
* User provided tags | |
*/ | |
this.tags = [] | |
} | |
newStream () { | |
if (!this.multiplexer) { | |
// await hasMultiplexerOrErrored() // magic ensues | |
} | |
// ... new stream creation | |
} | |
getStreams () { | |
} | |
close () { | |
this.timeline.closeTs = Date.now() | |
} | |
setLocalAddress (ma) { | |
this.endpoints.local = ma | |
} | |
} |
When does a Connection get created? My thought is this should be created once the initial transport dial is complete.
Agree
If so, this creates problems for the constructor here, because we only know the remoteAddress and isInitiator.
We would be able to immediately add the peerInfo after the dial, but the transport currently doesn't know it on dial.
Well, I am thinking about having a status OPENING
and UPGRADING
too. What do you think about the following:
const connection = _switch.dial(peerInfo) // Dials all transports and multiaddrs for the peer
await _switch.connectionUpgrader.privatize(connection)
const encryption = await _switch.connectionUpgrader.encrypt(connection) // `encryption` is negotiated
const multiplexer = await _switch.connectionUpgrader.multiplex(connection) // `multiplexer` is negotiated
connection.upgrade(encryption, multiplexer)
nextTick(() => _switch.identifyService.identify(connection)) // runs identify (this probably shouldn't block the return) and sets `localAddress`
return connection // Return the connection to the user
or
const connection = _switch.dial(peerInfo) // Dials all transports and multiaddrs for the peer
connection.upgrade(switch) // Upgrade connection
nextTick(() => _switch.identifyService.identify(connection)) // runs identify (this probably shouldn't block the return) and sets `localAddress`
return connection // Return the connection to the user
I am thinking that It will be useful for introspection to have the upgrading state. What do you think?
This shouldn't take a stream, right?
Yes, that should be a protocol! I will edit it
So, I think it would be ideal to have a ConnectionUpgrader
on the switch
, or expand the ConnectionManager
for this. It would be responsible for changing state on the connection. Right now, the ConnectionFSM
has a lot of logic around upgrading itself, which makes it a bit of a mess to follow.
I would expect the Upgrader/Manager
to also update any status codes on the connection. There are quite a few possible states for a connection and they get convoluted pretty quickly. I think having the simpler set of overall status, like you have is good. (Maybe change Active
to Open
?)
A sub status might be helpful, but I am trying to think of how they would best be used. We can get some information from introspection based on whether or not certain properties are set. Such as, an Open
connection with no encryption
or multiplexer
likely just opened and is in the process of encrypting. The most likely scenario I see is when we want a new stream from a Connection that has not yet upgraded with a multiplexer. But ideally, I think newStream
would just wait until the multiplexer was either added to the connection, or an error occurred, in which case newStream would error. It would be tricky logic but:
async newStream (protocol) {
if (!this.multiplexer) {
await hasMultiplexerOrErrored() // magic ensues
}
// ... new stream creation
}
Now that I am thinking of it, I don't think we need to save encryption
as a property. Do you see a need for it?
Ok, I agree with setting those properties by the responsible parties. Would you expect API methods for it, or directly modified? I am thinking about the best way to guarantee that it is not forgotten.
I updated the code gist with the new version according to the feedback.
Now that I am thinking of it, I don't think we need to save encryption as a property. Do you see a need for it?
Considering that we have a connEncryption
module in the js-libp2p
and we want to be able to see what encryption is being used in the introspection I think we need to keep it. Do you see a way to infer it?
Ok, I agree with setting those properties by the responsible parties. Would you expect API methods for it, or directly modified? I am thinking about the best way to guarantee that it is not forgotten.
This should be internal logic to the Switch, so we wouldn't have to worry about users setting it. Each new connection would get passed to the upgrader, and would end up with all properties set (assuming everything worked).
Considering that we have a connEncryption module in the js-libp2p and we want to be able to see what encryption is being used in the introspection I think we need to keep it. Do you see a way to infer it?
So would this just be the protocol for encryption then? ie: "/secio/1.0.0"
If so, I wonder if connection.tags
is a better place to record that?
So would this just be the protocol for encryption then? ie: "/secio/1.0.0" If so, I wonder if connection.tags is a better place to record that?
Yes, it would be the encryption protocol! I only prefer to have its own property because it should be always present and the tags would probably be an array of string with no context (the encryption should be always easy to get from the connection)
I only prefer to have its own property because it should be always present and the tags would probably be an array of string with no context
Fair enough. We can always revisit when we add more than 1 encryption. Right now if the switch has encryption turned on it's going to be secio
, otherwise it's plaintext
. Once tls or quic support is added it will become more important for this.
When does a
Connection
get created? My thought is this should be created once the initial transport dial is complete.If so, this creates problems for the constructor here, because we only know the
remoteAddress
andisInitiator
.We would be able to immediately add the
peerInfo
after the dial, but the transport currently doesn't know it on dial.localAddress
would be set after the Identify protocol runs and we get our observed address from the peer.encryption
&multiplexer
both need to be negotiated and would be set latertags
would also likely get set/added laterThis would let us update the switch connection upgrade logic (currently in ConnectionFSM) to something more like:
This shouldn't take a stream, right?