-
-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update libp2p to 0.30.x #1999
Update libp2p to 0.30.x #1999
Conversation
Code Climate has analyzed commit 7a076d9 and detected 0 issues on this pull request. View more on Code Climate. |
Sim test consistently fail, I've seen this in the logs
Not sure if any of these are related to the PR |
It seems like this is unique to this PR. It looks like the node is not able to stop properly (but just in the threaded sim test) |
The simulation test was failing due to libp2p/js-libp2p-tcp#139 which has been resolved and re-released. This PR is ready for re-review :) |
@@ -96,11 +96,11 @@ export class Libp2pNetwork extends (EventEmitter as {new (): NetworkEventEmitter | |||
public async stop(): Promise<void> { | |||
this.libp2p.connectionManager.removeListener(NetworkEvent.peerConnect, this.emitPeerConnect); | |||
this.libp2p.connectionManager.removeListener(NetworkEvent.peerDisconnect, this.emitPeerDisconnect); | |||
await Promise.all([this.diversifyPeersTask.stop(), this.checkPeerAliveTask.stop()]); |
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.
everything else in the PR looks good to me, but i'm curious, what is the effect of stopping these tasks here instead of where it used to be? What motivated this particular change?
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.
These tasks are for searching for peers and pinging peers. Before, we were shutting these down after we shut down libp2p, and no longer had any peers. So there was a slight change that tasks could error or not shut down cleanly.
This was usually not even visible or a practical issue, since these tasks run on long intervals, and would shut down before having a chance to run after libp2p had been shut down. During debugging this PR, this little wrinkle became apparent tho.
Having trouble committing on top of #1820
Updates libp2p dependencies to latest versions & updates yarn.lock