ethereum / py-evm

A Python implementation of the Ethereum Virtual Machine
https://py-evm.readthedocs.io/en/latest/
MIT License
2.25k stars 645 forks source link

Can Discovery be an isolated plugin? #1518

Closed pipermerriam closed 5 years ago

pipermerriam commented 5 years ago

What is wrong?

I think the DiscoveryService could be moved into it's own isolated process plugin.

How can it be fixed

I think the only functionality that the DiscoveryService needs with-respect-to external interactions is fulfilling requests for new node connection candidates.

Currently this is done by the DiscoveryService checking if the PeerPool is full if not, pushing these new connection candidates into the PeerPool. I think this should be inverted so that the PeerPool instead pulls new connection candidates from the DiscoveryService.

cburgdorf commented 5 years ago

I think the DiscoveryService could broadcast something like a NewPeerCandidateEvent event on the EventBus. This would allow any interested party to react upon these events.

If frequency of these events would cause too much traffic to broadcast them to each and everyone on the event bus, we can either throttle the event propagation or only send them to those that are listening for a specific topic (a feature that doesn't exist yet in the event bus but which is rather easy to implement). Something like:

event_bus.broadcast(
    NewPeerCandidate(candidate)
    BroadcastConfig(topic="peerstuff")
)
cburgdorf commented 5 years ago

Btw, this rhymes nicely with #1535. I think if we write a plugin for this it can probably be used in the regular trinity node as well as in the trinity beacon node.

Related: @jannikluhn @hwwhww do I understand that correctly that the current plan for the beacon node is to use discovery v5 for peer discovery but libp2p for the actual communication? Sorry if that doesn't make sense. I'm a total p2p noob.

jannikluhn commented 5 years ago

Related: @jannikluhn @hwwhww do I understand that correctly that the current plan for the beacon node is to use discovery v5 for peer discovery but libp2p for the actual communication? Sorry if that doesn't make sense. I'm a total p2p noob.

The way I imagine it right now is to use libp2p for

What we would have to implement in addition to that:

cburgdorf commented 5 years ago

Thanks @jannikluhn that makes sense!

cburgdorf commented 5 years ago

I think the DiscoveryService could broadcast something like a NewPeerCandidateEvent event on the EventBus. This would allow any interested party to react upon these events.

I think I back off from what I said there. I just looked a bit deeper into the discovery service and it works different from what I thought. I think @pipermerriam proposal makes sense and I'll give it a shot.