prysmaticlabs / prysm

Go implementation of Ethereum proof of stake
https://www.offchainlabs.com/
GNU General Public License v3.0
3.44k stars 979 forks source link

Advertise discv5 / ENR via libp2p multiaddress #5451

Open prestonvanloon opened 4 years ago

prestonvanloon commented 4 years ago

Jotting down this idea from an offline discussion with @nisdas.

Prysm nodes advertise several multiaddresses, however when a libp2p node peers with another without using discovery v5, then these nodes don't exchange their ENRs and the nodes learn about metadata through p2p rpc.

This idea proposes to advertise another multiaddress containing the ENR. Perhaps adding a new protocol to facilitate this functionality.

An example multiaddr might be

nisdas commented 4 years ago

For some related discussion on this https://github.com/ethereum/eth2.0-specs/issues/1637 and https://github.com/ethereum/eth2.0-specs/pull/1673. For most purposes having the ping/metadata rpc methods will be sufficient. As we just need to know the current status of subscribed subnets of our peers.

However being able to exchange enrs to other peers has advantages from the point of peer discovery. As that way you will be able build a larger dht, by adding new nodes to your local table.

However one downside of this is that ENRs are large objects being around 300 bytes. Constantly exchanging it whenever you connect with a new peer might be expensive.

prestonvanloon commented 4 years ago

With ethereum/eth2.0-specs#1637, we shouldn't need the above functionality. I recall from the offline discussion that we aren't adding ENR data from peers which weren't discovered via discv5. Is this still the case with the metadata rpc?

Do we need to leverage the metadata rpc to populate / maintain our discv5 peer table?

nisdas commented 4 years ago

Yes that it still the case, the metadata basically consists of the

type MetaData struct {
   SequenceNumber uint64
   SubnetBitfield bytes
   }

We mostly need the bitfield now from the metadata. However the metadata only gives us a portion of the enr. So we cannot use the metadata to add that peer to our discv5 table. There would always be a gap, as we aren't able to add that peer to our dht. So there are always peers we will miss out on.

rauljordan commented 4 years ago

@nisdas can we close this? or is it still relevant?

blazejkrzak commented 3 years ago

We should not miss any peers