server (socket-pairer) throws if client write() without noise

[LuKks]

Server throws:

buffer.js:560
      if (list[i].length) {
                  ^

TypeError: Cannot read property 'length' of null
    at Function.concat (buffer.js:560:19)
    at Socket.onreadable (/self/hyperforward/node_modules/@hyperswarm/dht/lib/socket-pairer.js:606:36)
    at Socket.emit (events.js:400:28)
    at emitReadable_ (internal/streams/readable.js:555:12)
    at onEofChunk (internal/streams/readable.js:533:5)
    at readableAddChunk (internal/streams/readable.js:247:5)
    at Socket.Readable.push (internal/streams/readable.js:206:10)
    at TCP.onStreamRead (internal/stream_base_commons.js:238:12)

client.js

const DHT = require('@hyperswarm/dht')
const net = require('net')

const serverKeyPair = DHT.keyPair(Buffer.from('524ad00b147e1709e7fd99e2820f8258fd30ed043c631233ac35e17f9ec10333', 'hex'))
const clientKeyPair = DHT.keyPair(Buffer.from('c7f7b6cc2cd1869a4b8628deb49efc992109c9fbdfa55ab1cfa528117fff9acd', 'hex'))

const node = new DHT({ keyPair: clientKeyPair })

const peer = node.connect(serverKeyPair.publicKey)
peer.on('open', function () {
  console.log('peer', peer.rawStream.remoteAddress + ':' + peer.rawStream.remotePort)

  const socket = net.connect(peer.rawStream.remotePort, peer.rawStream.remoteAddress)
  socket.end('hey')
})

server.js

const DHT = require('@hyperswarm/dht')
const net = require('net')

const serverKeyPair = DHT.keyPair(Buffer.from('524ad00b147e1709e7fd99e2820f8258fd30ed043c631233ac35e17f9ec10333', 'hex'))
const clientKeyPair = DHT.keyPair(Buffer.from('c7f7b6cc2cd1869a4b8628deb49efc992109c9fbdfa55ab1cfa528117fff9acd', 'hex'))

const node = new DHT({ keyPair: serverKeyPair })

const server = node.createServer({
  firewall: function (remotePublicKey, remoteHandshakePayload) {
    console.log('on firewall, public key:\n' + remotePublicKey.toString('hex'))
    return !remotePublicKey.equals(clientKeyPair.publicKey)
  }
})

server.on('connection', function (peer) {
  console.log('peer', peer.rawStream.remoteAddress + ':' + peer.rawStream.remotePort)
})

server.listen(serverKeyPair).then(() => console.log('server listening'))

It only throws if the client can reach server's ip:port, for example, listen and connect in the same local network or my machine as client -> DO as server (DO has ports open).

Aside from the issue, I just started experimenting with P2P so obviously I'm missing a lot but DHT should be able to connect(port, address) at will, while there is an active peer connection that keeps alive the initial holepunch.

In case two peers already knows each other ip:port (like two friends wanting to connect), it would be great to have a holepunch(port[, address]) and connect(port[, address]) to avoid querying bootstrap nodes and making direct connections.

Also, it's not crazy to think about having optional proxy support for bootstrap queries and connect() (which can fallback to relay servers and in that case it's ok), if an user trusts his proxy (like ProtonVPN or Tor) then it would extremely increase the DHT privacy and it can attract a lot of new users.

ProtonVPN has some P2P proxies, I never tested them but maybe in that specific case holepunch works and won't fallback to relay servers. I tested once a Plus proxy from Proton (which is paid) but the connection doesn't happen, that's why I said it would fallback to relays.

[LuKks]

Maybe it's redundant but notice that, not just proxy but the direct holepunch/connect will increase privacy.

[mafintosh]

Thanks, sounds like a bug. Btw keep the issue focused around a single bug / issue, otherwise it's hard to track things :)

[mafintosh]

Fixed in 5.0.5, thanks!

[mafintosh]

To answer your other question, we are not doing to support connect(port, host) as a dht api, as all peers are identified by pub keys and that's bootstrapping default encryption / authentication. That signature also won't translate to holepunching so will only work for non firewalled machines, ie the cloud.

You can run your own server that you discover through the DHT if that's important to you (and we might add some classic find (host, port) pairs through a lookup later on)

[mafintosh]

Bootstrap nodes are only hit for discovery of other initial nodes. It's an ultra cheap operation. Holepunching is done through other nodes in the dht itself (you ALWAYS need another node for holepunching).

[LuKks]

Thanks, sounds like a bug. Btw keep the issue focused around a single bug / issue, otherwise it's hard to track things :)

Sorry, you're right.

To answer your other question, we are not doing to support connect(port, host) as a dht api, as all peers are identified by pub keys and that's bootstrapping default encryption / authentication. That signature also won't translate to holepunching so will only work for non firewalled machines, ie the cloud.

I was trying to connect(port, host) + somehow noise-peer and expecting that to work (wanted to receive a connection even with blocked ports), but I was on limited sleep so I created the issue and I went to sleep lol.

You can run your own server that you discover through the DHT if that's important to you (and we might add some classic find (host, port) pairs through a lookup later on)

Bootstrap nodes are only hit for discovery of other initial nodes. It's an ultra cheap operation.

Right, it would be awesome having in the README how it starts from the peer A -> bootstrap nodes -> initial nodes -> lookup -> holepunch -> peer B (I don't actually know the path, it's an example), thay way everyone can have a sense of how it works.

Holepunching is done through other nodes in the dht itself (you ALWAYS need another node for holepunching).

Ooh, okay, I didn't know that you need another node for holepunching, I'll try to learn more!

Really thanks for all the responses and explanation, very helpful :)

[LuKks]

Instead of socket.end('hi'), doing socket.write() also crashes the server (in this case, the client crashes as well).

Client:

internal/streams/writable.js:285
      throw new ERR_INVALID_ARG_TYPE(
      ^

TypeError [ERR_INVALID_ARG_TYPE]: The "chunk" argument must be of type string or an instance of Buffer or Uint8Array. Received undefined
    at new NodeError (internal/errors.js:322:7)
    at Socket.Writable.write (internal/streams/writable.js:285:13)
    at NoiseSecretStream.<anonymous> (/self/hyperforward/cl2.js:14:10)
    at NoiseSecretStream.emit (events.js:400:28)
    at ReadableState.afterOpen (/self/hyperforward/node_modules/streamx/index.js:493:12)
    at NoiseSecretStream._open (/self/hyperforward/node_modules/@hyperswarm/secret-stream/index.js:296:14)
    at NoiseSecretStream.start (/self/hyperforward/node_modules/@hyperswarm/secret-stream/index.js:96:12)
    at Pair.c.pair.onconnection (/self/hyperforward/node_modules/@hyperswarm/dht/lib/connect.js:210:23)
    at Pair._ontcpconnection (/self/hyperforward/node_modules/@hyperswarm/dht/lib/socket-pairer.js:272:10)
    at Socket.onconnect (/self/hyperforward/node_modules/@hyperswarm/dht/lib/socket-pairer.js:464:12) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Server:

events.js:377
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at TCP.onStreamRead (internal/stream_base_commons.js:209:20)
Emitted 'error' event on NoiseSecretStream instance at:
    at WritableState.afterDestroy (/self/hyperforward/node_modules/streamx/index.js:442:19)
    at NoiseSecretStream._destroy (/self/hyperforward/node_modules/@hyperswarm/secret-stream/index.js:365:5)
    at WritableState.updateNonPrimary (/self/hyperforward/node_modules/streamx/index.js:189:16)
    at WritableState.update (/self/hyperforward/node_modules/streamx/index.js:174:70)
    at WritableState.updateWriteNT (/self/hyperforward/node_modules/streamx/index.js:482:8)
    at processTicksAndRejections (internal/process/task_queues.js:77:11) {
  errno: -104,
  code: 'ECONNRESET',
  syscall: 'read'
}

I used npm i hyperswarm/dht because installing from npm I don't see the new test in test/connections.js, is there something wrong on my side?

[LuKks]

Just in case, this is the client:

const DHT = require('@hyperswarm/dht')
const net = require('net')

const serverKeyPair = DHT.keyPair(Buffer.from('524ad00b147e1709e7fd99e2820f8258fd30ed043c631233ac35e17f9ec10333', 'hex'))
const clientKeyPair = DHT.keyPair(Buffer.from('c7f7b6cc2cd1869a4b8628deb49efc992109c9fbdfa55ab1cfa528117fff9acd', 'hex'))

const node = new DHT({ keyPair: clientKeyPair })

const peer = node.connect(serverKeyPair.publicKey)
peer.on('open', function () {
  console.log('peer', peer.rawStream.remoteAddress + ':' + peer.rawStream.remotePort)

  const socket = net.connect(peer.rawStream.remotePort, peer.rawStream.remoteAddress)
  socket.write()
})

The server code is the same.

[LuKks]

Same effect by doing peer.write() (no arguments as before like socket.write()) but it could be the same cause.

[LuKks]

Same effect by doing peer.emit('error', new Error('random')).

[LuKks]

Oh, it's not that emitting an error on the peer makes the server crash, it's the ECONNRESET. So, just doing throw new Error('random') looks the same than doing peer.emit('error', ..).

[LuKks]

Sorry for too much comments. I was wondering why CTRL+C somehow closes gracefully and it doesn't have the ECONNRESET, but process.exit() does.

Edit: so, peer.write() does nothing special, it's just that the client process crashes and it's the same as doing process.exit().

Edit 2: My bad, the server just need to handle errors as usual: peer.on('error', console.error) So the ECONNRESET is not considered a bug, right?

[mafintosh]

Ya that’s expected. You need to error handle. Very much send me other crashes you find that are unhandled though :)