Skip to content

Instantly share code, notes, and snippets.

@vasco-santos
Last active May 13, 2019 15:30
Show Gist options
  • Save vasco-santos/bc89fe6e0acf8186f1137576135fbd49 to your computer and use it in GitHub Desktop.
Save vasco-santos/bc89fe6e0acf8186f1137576135fbd49 to your computer and use it in GitHub Desktop.
js-libp2p connection
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
}
}
@jacobheun
Copy link

jacobheun commented May 10, 2019

When does a Connection get created? My thought is this should be created once the initial transport dial is complete.

const connection = transport.dial(multiaddr)

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.

  • 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 later
  • tags would also likely get set/added later

This would let us update the switch connection upgrade logic (currently in ConnectionFSM) to something more like:

  const connection = await _switch.dial(peerInfo) // Dials all transports and multiaddrs for the peer
  await _switch.connectionUpgrader.privatize(connection)
  await _switch.connectionUpgrader.encrypt(connection)  // `encryption` is negotiated and set
  await _switch.connectionUpgrader.multiplex(connection) // `multiplexer` is negotiated and set
  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

newStream (stream) {

This shouldn't take a stream, right?

@vasco-santos
Copy link
Author

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

@jacobheun
Copy link

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?

@vasco-santos
Copy link
Author

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?

@jacobheun
Copy link

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?

@vasco-santos
Copy link
Author

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)

@jacobheun
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment