-
Notifications
You must be signed in to change notification settings - Fork 28
level up libp2p functionality #9
Conversation
This PR moves a lot of the libp2p maintenance that was being done inside js-ipfs to libp2p. It also brings features such as dialing by Multiaddr and PeerInfo to libp2p interface + the ability to hangup on a peer that we have a muxedConn open. Another new thing is a test that shows how a libp2p.Node crash doesn't translate in a uncaught exception in our node |
@dignifiedquire can you give CR? I'm still going to update libp2p-ipfs-browser and then js-ipfs-bitswap and js-ipfs to follow this new interface. |
will review in detail either later tonight or tomorrow |
this.peerBook.removeByB58String(peerInfo.id.toB58String()) | ||
}) | ||
|
||
this.isOnline = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would hide this to make sure it's not accidentally changed from the outside. Easiest to do with a getter and a locally scoped variable.
7970f3b
to
94d8435
Compare
spawnedNode.on('close', (code) => { | ||
// console.log(`child process exited with code ${code}`) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is being tested here? An expectation or an assert would help to convey the purpose. If it's WIP, perhaps add // WIP
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test spawns libp2pNode in a different process, which gets killed in the next test. I used this event listeners for debugging and left them here. I can remove them :)
Couple of stylistic comments on the test. I can't be 100% sure about the logic of the code as I'm unfamiliar with the internals of libp2p, but looks good to me. |
rad, thank you @haadcode and @dignifiedquire :) |
No description provided.