Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

"jsipfs swarm peers" listening all known address instead of used address to connect to that peer #956

Closed
victorb opened this issue Aug 25, 2017 · 4 comments
Assignees
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up status/in-progress In progress

Comments

@victorb
Copy link
Member

victorb commented Aug 25, 2017

  • Version: 0.25.0
  • Platform: macOS
  • Subsystem: libp2p

Type: Bug

Severity: High

Description:

swarm peers command (on CLI at least) seems to not be returning the right peers. First problem is that every peer that is connected is being added with each listener, for example, node QmVeWHZtuF2fMWDNF8ef2XoToLdyzuH9nwBexKqi4FmXop would show up as:

/ip4/127.0.0.1/tcp/4002/ipfs/QmVeWHZtuF2fMWDNF8ef2XoToLdyzuH9nwBexKqi4FmXop
/ip4/10.185.197.146/tcp/4002/ipfs/QmVeWHZtuF2fMWDNF8ef2XoToLdyzuH9nwBexKqi4FmXop
/ip4/127.0.0.1/tcp/4003/ws/ipfs/QmVeWHZtuF2fMWDNF8ef2XoToLdyzuH9nwBexKqi4FmXop
/ip4/127.0.0.1/tcp/4002/ipfs/QmVeWHZtuF2fMWDNF8ef2XoToLdyzuH9nwBexKqi4FmXop
/ip4/10.185.197.146/tcp/4002/ipfs/QmVeWHZtuF2fMWDNF8ef2XoToLdyzuH9nwBexKqi4FmXop

Instead of just being listed once as expected. Which means swarm peers | wc -l will return addresses the node is connected to, instead of peers.

Steps to reproduce the error:

  • Initialize two nodes
  • Start one of the nodes
  • Print out jsipfs swarm peers and jsipfs swarm peers | wc -l, note down the number
  • Start the other node and wait five seconds for them to connect automatically
  • Print out jsipfs swarm peers and jsipfs swarm peers | wc -l, compare with previous number
@victorb victorb added the kind/bug A bug in existing code (including security flaws) label Aug 25, 2017
@victorb victorb changed the title "jsipfs swarm peers" returning wrong data "jsipfs swarm peers" returning connected addresses instead of connected peers Aug 25, 2017
@daviddias daviddias changed the title "jsipfs swarm peers" returning connected addresses instead of connected peers "jsipfs swarm peers" listening all known address instead of used address to connect to that peer Aug 25, 2017
@daviddias
Copy link
Member

daviddias commented Aug 25, 2017

Seems that issue is here:

peers: promisify((opts, callback) => {
if (typeof opts === 'function') {
callback = opts
opts = {}
}
if (!self.isOnline()) {
return callback(OFFLINE_ERROR)
}
const verbose = opts.v || opts.verbose
// TODO: return latency and streams when verbose is set
// we currently don't have this information
const peers = values(self._peerInfoBook.getAll())
.filter((peer) => peer.isConnected())
const peerList = flatMap(peers, (peer) => {
return peer.multiaddrs.toArray().map((addr) => {
const res = {
addr: addr,
peer: peer
}
if (verbose) {
res.latency = 'unknown'
}
return res
})
})
callback(null, peerList)
}),

The filtering is being done by asserting if we are connected, when instead we should be picking the used address from that same call -- see https://github.com/libp2p/js-peer-info/blob/6fe1352ec9105b3e79c269d7c4b668ec0322d995/src/index.js#L32-L34 --.

Looks like a small patch, wanna push a PR?

@daviddias daviddias added P2 Medium: Good to have, but can wait until someone steps up exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue labels Aug 25, 2017
@victorb
Copy link
Member Author

victorb commented Aug 25, 2017

@diasdavid I'll be happy to :)

@victorb victorb self-assigned this Aug 25, 2017
@daviddias daviddias added the status/in-progress In progress label Aug 28, 2017
@victorb
Copy link
Member Author

victorb commented Sep 1, 2017

Since #960 was merged, this has been fixed

@victorb victorb closed this as completed Sep 1, 2017
@daviddias
Copy link
Member

wwoot! Thank you @victorbjelkholm for handling this one :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up status/in-progress In progress
Projects
None yet
Development

No branches or pull requests

2 participants