feat(discv5): open dns for discv5

[emhane]

To add a node to sigp/discv5, a signed node record is needed.

reth/crates/net/network/src/discovery.rs

Lines 210 to 215 in 7efd5b0

	while let Some(Poll::Ready(Some(update))) =
	self.dns_discovery_updates.as_mut().map(|updates| updates.poll_next_unpin(cx))
	{
	self.add_discv4_node(update.node_record);
	self.on_node_record_update(update.node_record, update.fork_id);
	}

Alternative solutions: parse unsigned node record into libp2p Multiaddr type and request its ENR again. Why not use this solution? Since the dns service is already handling type Enr<SecretKey>, it's unnecessary to make the extra RTT to re-request the peer's ENR, before the peer can be used in a lookup query.

Sequel: Since dns/resolver already is generic over ENR key type, it makes sense to generalise the whole dns crate w.r.t. key type. This would save the extra work of encoding an Enr<SecretKey> and decoding it as an Enr<CombindeKey>, which is the type sigp/discv5 uses. Alt: address sigp/discv5#233.

[mattsse]

isn't the only thing we need a new enr field in the DnsNodeRecordUpdate?

so that we could also pipe the enrs to discv5 here:

reth/crates/net/network/src/discovery.rs

Lines 210 to 213 in 28935da

	while let Some(Poll::Ready(Some(update))) =
	self.dns_discovery_updates.as_mut().map(|updates| updates.poll_next_unpin(cx))
	{
	self.add_discv4_node(update.node_record);

[emhane]

isn't the only thing we need a new enr field in the DnsNodeRecordUpdate?

so that we could also pipe the enrs to discv5 here:

reth/crates/net/network/src/discovery.rs

Lines 210 to 213 in 28935da

	while let Some(Poll::Ready(Some(update))) =
	self.dns_discovery_updates.as_mut().map(|updates| updates.poll_next_unpin(cx))
	{
	self.add_discv4_node(update.node_record);

didn't want to break the public api of the crate, thoughts?

[mattsse]

imo this would be a very simple change and gets us exactly what we want.

no concerns re breaking changes

[emhane]

imo this would be a very simple change and gets us exactly what we want.

no concerns re breaking changes

in that case I wouldn't strip the ENR of its signature at all, i.e. don't convert from Enr type to NodeRecord type, it is only done to pass the node record up to the app anyway. will this change be welcomed?

[emhane]

re-request review @mattsse @Rjected

[mattsse]

lgtm,

optional suggestion for error variant,
not super happy with the NodeRecord type in general because the only reason we have it under rpc is that we can reuse it in a few rpc functions

but we'll try to push it to alloy so we don't have to deal with it here anymore

ref alloy-rs/alloy#405

[mattsse]

lgtm