Add MetadataV3 with custody_subnet_count

[dapplion]

When a node receives an inbound connection from another node it does not have a way to learn its custody_subnet_count. Currently, this value is only propagated via ENRs, so it's only guaranteed to be available in the outbound connection code-path.

It's important to learn the custody_subnet_count of all peers to discover and utilize supernodes.

[g11tech]

lgtm

[cskiraly]

Makes sense to extend the Metadata with this.

I do wonder however why there is no GetENR among the req/resp messages. Wouldn't that be useful to avoid the need for these updates to the Metadata?

[dapplion]

I do wonder however why there is no GetENR among the req/resp messages. Wouldn't that be useful to avoid the need for these updates to the Metadata?

Because the ENR changes and adds keys we need strict versioning and a way to serialize the ENR as SSZ. Otherwise, we should stream an SSZ list of bytes with the ENR encoded as RLP.

[nisdas]

Is there any reason you can't use gossipsub for this ? Not against this PR, but the gossipsub router tracks all connected peers and their subscribed topics. So once you are connected to a peer, you simply need to check the router for which topics a particular peer is subscribed to without requiring its custody count. This would save you a network request which might becomes a bottleneck if you have to determine this for many peers in a short amount of time.

[dapplion]

So once you are connected to a peer, you simply need to check the router for which topics a particular peer is subscribed to without requiring its custody count

There's some asynchrony there, as the peer may send the subscriptions later. You are right that both pieces of information should match. I recall Nimbus having some logic to penalize peers whose Gossipsub subscriptions do not match their ENR.

[jimmygchen]

Looks good!

[ralexstokes]

I think this makes sense to include w/ the peerdas work

it looks like you changed the electra set of files -- can we update the EIP-7594 file instead?

[jimmygchen]

Is there any reason you can't use gossipsub for this ? Not against this PR, but the gossipsub router tracks all connected peers and their subscribed topics. So once you are connected to a peer, you simply need to check the router for which topics a particular peer is subscribed to without requiring its custody count. This would save you a network request which might becomes a bottleneck if you have to determine this for many peers in a short amount of time.

I think the downside with looking only at subscribed topics is that we assume that peers are all honest and subscribe to their custody subnets. With custody count, we'll be able to identity what the nodes are supposed to custody, and still send sampling requests to them (even if they're not subscribed for some reasons) and penalise them accordingly if they can't serve the requests.

[wemeetagain]

I do wonder however why there is no GetENR among the req/resp messages.

we should stream an SSZ list of bytes with the ENR encoded as RLP.

Is this a crazy idea? For peers who have dialed us, we won't have discovery port information, so can't easily query the peer's discv5 for its ENR. This potentially could be useful there too.

[ppopth]

This looks good to me. Just make the MetaData the same as the ENR.

[ppopth]

I do wonder however why there is no GetENR among the req/resp messages. Wouldn't that be useful to avoid the need for these updates to the Metadata?

@cskiraly You can already do the GetENR thing using devp2p (not libp2p). See https://github.com/ethereum/devp2p/blob/master/discv5/discv5-wire.md#findnode-request-0x03

You can do FINDNODE with distance 0

[nalepae]

For information we implemented it in Prysm: prysmaticlabs/prysm#14274

[hwwhww]

lgtm. thanks!