Skip to content
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

Improve p2p peer management #365

Open
kdeme opened this issue Jun 15, 2021 · 2 comments
Open

Improve p2p peer management #365

kdeme opened this issue Jun 15, 2021 · 2 comments

Comments

@kdeme
Copy link
Contributor

kdeme commented Jun 15, 2021

Issue for adding/improving proper peer management to the p2p code.
This currently exists out of a very basic peer pool and some helper calls to launch rlpx protocols and discovery.

Should be improved to have at least:

  • Review current API and easy of set-up (Proper API to use async safely?)
  • re-connect to staticnodes / bootnodes provided
  • incoming / outgoing peer limitations
  • ...?

This would be mostly for nimbus-eth1 so it might also go in that repository. However, waku v1 also uses this code right now.

Edit:
Some old outstanding issues that could be considered sub-issues:

And same issue exists for Waku v1 basically: waku-org/nwaku#18

@jlokier
Copy link
Contributor

jlokier commented Jun 15, 2021

Periodic re-connect to staticnodes is essential when discovery is disabled. At the moment most of my testing is with staticnodes only, so it is very noticable when a connection is rejected and then Nimbus Eth1 doesn't retry. It just remains stuck with zero peers.

I would rather static nodes were treated similarly to dynamically discovered nodes, as "currently disconnected peers" in the peer table. In both cases, period re-connection attempts are worth doing, with the difference being static nodes remain always in the table, while dynamically discovered nodes are dropped.

Thanks for mentioning waku v1. I have been looking at nimbus-eth2 to see which parts of nim-eth seem to be exclusively used by nimbus-eth1, and therefore fair game for changing to fit nimbus-eth1's needs better. I didn't realise there are other projects also using nim-eth. If it's also used by waku v1 that complicates changes and decision making.

@kdeme
Copy link
Contributor Author

kdeme commented Sep 20, 2021

Thanks for mentioning waku v1. I have been looking at nimbus-eth2 to see which parts of nim-eth seem to be exclusively used by nimbus-eth1, and therefore fair game for changing to fit nimbus-eth1's needs better. I didn't realise there are other projects also using nim-eth. If it's also used by waku v1 that complicates changes and decision making.

Yeah, however the current peer/connection management is pretty much non-existent. A re-write seems appropriate, and I understand that in order to avoid breaking or adding complexity you might want to do it in nimbus-eth1.
However in order for Waku v1 to benefit from this, it would be better to have it in nim-eth.

The question is whether it is needed for Waku v1. The Waku v1 node is not in production. I'm not sure what the plan is however with the Waku v1 to v2 bridge. In any case, it will also be handy to have a better connection management in there, but would require somebody of the Waku team the fix items on the waku side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants