js-libp2p connection

connection.js

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
  }
}

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.