sigp / lighthouse

Ethereum consensus client in Rust
https://lighthouse.sigmaprime.io/
Apache License 2.0
2.97k stars 769 forks source link

Shared global ENR #4706

Open AgeManning opened 1 year ago

AgeManning commented 1 year ago

Description

Discv5 has a number of services. It maintains an Arc<RwLock< ENR>> which passes it around the different services.

Lighthouse also has a global network variable list. Inside it, holds a RwLock<< ENR>> and the global variable list is usually Arc'd around between different threads.

We currently have the situation where we are storing two kinds of Enr in memory. One (that actual source of truth) lies inside of discv5. The second, the one Lighthouse knows about, lies in the NetworkGlobals.

We try and keep these in sync by updating our view based on messages we receive from discovery. However, I think a better design is to just keep the one record and share mutable access across Lighthouse and discovery.

The benefits I think are:

Downsides are:

Implementation:

I think to implement this we need to expose the local ENR in the discv5 struct (in the discv5 repo). Once exposed, we need to build it for discovery and then store the resulting reference in the NetworkGlobals.

There are some immediate simplifications we can make once doing this. I'd start with removing functions like this: https://github.com/sigp/lighthouse/blob/stable/beacon_node/lighthouse_network/src/discovery/mod.rs#L405 and probably the calling code.

jmcph4 commented 1 year ago

Strong agree and likely makes sense to get this in prior to #4675.