feat(discv5): plug discv5 crate into network #7446
Conversation
|
ENR and secp256k1 is updated here: #7000 |
crates/net/network/src/discovery.rs
Outdated
| Discv4::bind(discovery_addr, local_enr, sk, disc_config).await.map_err(|err| { | ||
| NetworkError::from_io_error(err, ServiceKind::Discovery(discovery_addr)) | ||
| })?; | ||
| Discv4::bind(discovery_v4_addr, local_enr, sk, disc_config).await.map_err( |
There was a problem hiding this comment.
I get your point that this setup is now even more convoluted.
this is okay for this PR.
but we could make this a bit nicer separately and run the two setups concurrently.
perhaps a match over both configs would be "more" ergonomic here
There was a problem hiding this comment.
on the down side, when running both versions, this means a significant overhead for the reth node. it will be having double up discovery connections, once on each version, for one unique mempool peer. on the bright side, from an ethereum wide perspective, it will significantly help peer discovery, since finding specific peers becomes easier the more well connected nodes exist in the network. for example DAS will prosper form this.
not sure if it improves readability, since the two versions now have no dependency on each other, i.e. discv4 is run the same with discv5 as without discv5 and vv.
mattsse
added
A-networking
Discv5inreth-network.Shortcoming:
Unable to update discv5 fork id out of the box. Sigp/discv5 only exposes method
enr_insert,which internally rlp encodes the given value. Conversion fromsince this doesn't effect any other network than L1 atm. This is becauseForkIdinto BE bytes must be implemented, I started this but moved it to another branch,ForkIdupdate isn't configured for other keys than'eth', and fixing that would be out of scope.update: sigp/discv5 has moved to
alloy_rlpon master