Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Basic peer manager based on libp2p
peer_store
andconnection_limits
#126Basic peer manager based on libp2p
peer_store
andconnection_limits
#126Changes from all commits
48d7fbb
0a76a80
9e9e485
c6c8176
eeed3a0
b21082f
6057fd2
0b2a1e1
f629645
69133ea
95c982a
cbfd49c
86625a0
4c494c0
ccd7c99
c34c4d8
14149e3
73c81bd
32b7510
6d59ee3
76cb0d6
dc4a881
55a6a60
372a096
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
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.
interesting! Didn't know one could this, I suggest we use our own fork or yours Daniel, as Dr Huang may rebase and I fear that commit may disapear, wdyt?
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 don't think that can happen. The commit can disappear from the branch, but it stays in the repo if it was pushed. For example this commit is still accessible via the link: sigp/lighthouse@7f33d35, even though it no longer is in the current commit chain of the PR branch.
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.
ok thanks for confirming Daniel :)
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.
Lighthouse does something like that https://github.com/sigp/lighthouse/blob/unstable/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs#L113
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.
The event only allows us to dial a single peer.
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.
Yes, I think it creates an event for each peer. But honestly, I don't know the reason. Maybe decoupling?
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.
@jxs: Is there a relevant difference between these approaches to dial?
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.
We could also encapsulate this code inside the Peer Manager. Similar to https://github.com/sigp/lighthouse/blob/stable/beacon_node/lighthouse_network/src/service/mod.rs#L1845
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.
sorry, missed this!
no, one calls the other underneath :)