Unexpected Deduplication behavior. Is it supposed to work like this?

[martinheidegger]

Working on a PR for adding holepunchto/hypercore#291 to corestore-networker I noticed that on my local network two streams per local instance get opened (4 streams for 2 instances, per instance: one for peer: <IPV4> and one for peer ::ffff:<IPV4>). This seems like a bug but I am not sure.

(Test code):

  const { networker: networker1, store: store1 } = await create()
  const { networker: networker2, store: store2 } = await create()
  const core1a = store1.get()
  await append(core1a, 'a')
  const core2a = store2.get({ key: core1a.key })

  networker1.on('peer-add', () => console.log('nw1-add'))
  networker2.on('peer-add', () => console.log('nw2-add'))

  await networker1.configure(core1a.discoveryKey)
  await networker2.configure(core2a.discoveryKey)

(of the two outputs nw1-add and nw2-add it will output one twice and one once)

Looking at the duplication logic the two streams have indeed the same id, so the second stream on both instances triggers: https://github.com/hyperswarm/hyperswarm/blob/e4c51a407fb9e0e8fdcece461fc4917c2d5b23d2/lib/queue.js#L89-L91

and particularly the second line seems strange: cmp < 0. This means that one of the instances always returns true while the other always returns false as the localId is randomly generated. This means that on one instance the second stream will be identified as "duplicate" while on the other is not.

As a consequence we have two peers added on one instance while we have only one peer added on the second. Does that seem right?

[mafintosh]

Are the duplicate peers not getting dropped? I see you're only tracking peer-add

[mafintosh]

If you can post a full example, I can also try to see myself.

[martinheidegger]

@mafintosh It is testing for peer-add, indeed. As far as I can tell: the stream is opened by corestore networker because the deduplicate option returns false but late closed because the deduplication always happens at least at one peer. That being said: opening the stream shouldn't be done in the first place, right?

Here is an example to reproduce the behavior:
https://gist.github.com/martinheidegger/4e4a08e101aa603fc1bc9d7527fa392d

Current Output (on my machine):

> Creating stores
> Appending to first core
> Configuring first networker
> Configuring second networker
nw1-add 192.168.1.2
nw2-add 192.168.1.2
nw1-add ::ffff:192.168.1.2
nw1-stream-closed
nw2-stream-closed
> Just for test: see if waiting a bit is relevant before closing
> Closing networker 1
nw1-stream-closed
nw2-stream-closed
nw2-stream-closed
nw2-stream-closed
nw2-stream-closed
nw2-stream-closed
nw2-stream-closed
nw2-stream-closed
> Closing networker 2
> Closing store 1
> Closing store 2
> Done.

Note: That the second stream is closed all that often seems unintuitive.

[mafintosh]

No that's supposed to happen. It doesn't know it needs dedup until it has the second dup - that's why the second stream closes after as well. with the key addressing scheme in the swarm upgrade this is a bit easier to deal with upfront thankfully so hopefully down the line we can make that simpler.

[martinheidegger]

Thank you for checking it. I am looking forward to the upgrade.