libp2p / js-peer-info

libp2p Peer abstraction Node.js implementation
https://libp2p.io
MIT License
37 stars 28 forks source link

peerinfo generates wrong connected multiaddr when using discovery in browser #68

Closed gitGksgk closed 4 years ago

gitGksgk commented 6 years ago

this is pretty hard to reproduce, some behavior is still not comprehended .I'll try to make myself understood anyway... set up several nodes ( go node, js node, virtual node, nodes in different computers), don't bootstrap to outside network (so as to get a clear experiment environment). enable dht and circuits in js-ipfs. then nodes will discover and connect to each other. now set up a browser node with dht and circuits enabled. The node will discover others too once connected to a go node in the same computer. Now, the issue comes here: while it returns correct CIDs, the multiaddrs are very arbitrary. sometimes it considers another computer's connected multiaddr as 127.0.0.1. sometimes it returns address like 169.254.65.29, an unknown unreachable address.. sometimes it will get a correct multiaddr of another computer though. I've looked into the code:

0: {addr: ClassIsWrapper, peer: PeerId}
1: {addr: ClassIsWrapper, peer: PeerId}
2: {addr: ClassIsWrapper, peer: PeerId}
3: {addr: ClassIsWrapper, peer: PeerId}
4: {addr: ClassIsWrapper, peer: PeerId}
5: {addr: ClassIsWrapper, peer: PeerId}
6: {addr: ClassIsWrapper, peer: PeerId}
7: {addr: ClassIsWrapper, peer: PeerId}
8: {addr: ClassIsWrapper, peer: PeerId}

it shows that the browser node has discovered 8 multiaddrs. the peerId is perfectly correct. addr is perfectly messy. When digging deeper into swarm.js I found that in _peerInfoBook, it has

QmYV8tMHxfymR1NNRhfUxvWtHou3YHExd6AZt5tdMcH9Wa: PeerInfo
id: PeerId {_id: Uint8Array(34), _idB58String: "QmYV8tMHxfymR1NNRhfUxvWtHou3YHExd6AZt5tdMcH9Wa", _privKey: undefined, _pubKey: undefined}
multiaddrs: MultiaddrSet
**_multiaddrs: (7) [ClassIsWrapper, ClassIsWrapper, ClassIsWrapper, ClassIsWrapper, ClassIsWrapper, ClassIsWrapper, ClassIsWrapper]**
_observedMultiaddrs: []
size: (...)
__proto__: Object
protocols: Set(0) {}
**_connectedMultiaddr: ClassIsWrapper**
buffer: Uint8Array(8) [4, 192, 168, 197, 1, 6, 15, 161]
Symbol(@multiformats/js-multiaddr/multiaddr): true
Symbol(Symbol.toStringTag): (...)
__proto__: withIs.proto.className
__proto__: Object

it shows that in a certain discovered node we have found 7 multiaddrs and have got 1 connectedMultiaddr. in 7 multiaddrs there's often a correct ip address of that discovered not, though not always becoming the connectedMultiaddr, which eventually become messy addrs as mentioned above. In this example I got an addr of 192, 168, 197, 1 which is my vmware's ip, but the CID shows that the node is on another computer and has no chance to shake hands with my vmware. I'm not sure if this issue should be posted here or somewhere related to dht.. using peer-info@0.14.1

it seems that in peer-info\src\index.js there's connect(ma){...} function which adds the first (or random) address in each discovered node's multiaddr list, no matter it's actually connected or not.

vasco-santos commented 4 years ago

Thanks for reporting this @gitGksgk

We are deprecating this module per libp2p/js-peer-info#85. This should not be a problem anymore with js-libp2p new connection manager.

I am closing this. Let me know if this is still a problem in the new codebase.