discv5 support #5576
discv5 support #5576
Conversation
this should not be required, everything we need on top of discv5 should go into the reth-discv5 crate |
There was a problem hiding this comment.
okay after looking at an example:
I think all of this should be easier than expected, we perhaps don't even need the additional crate since the start function spawns the services.
what we should do here is
- Configure discv5
call start- get an event stream
- Store
Arc<Discv5>internally as field and the eventstream as field
|
@rkrasiuk let me know what additional changes need to be done. |
| @@ -31,7 +32,7 @@ reth-rpc-types.workspace = true | |||
| reth-tokio-util.workspace = true | |||
|
|
|||
| alloy-rlp.workspace = true | |||
|
|
|||
| k256 = "0.13.2" | |||
There was a problem hiding this comment.
rkrasiuk
added
C-enhancement
crates/net/network/src/discovery.rs
Outdated
| // Use a placeholder for `discv5` since it doesn't implement Debug | ||
| .field("discv5", &format_args!("<Discv5>")) |
There was a problem hiding this comment.
let's make our own wrapper around discv5 that implements Debug and export that from the crate? @mattsse thoughts?
There was a problem hiding this comment.
sg!
then we can keep reth-discv5 and have that type there @supernovahs
crates/net/network/src/discovery.rs
Outdated
| Event::Discovered(enr) => {} | ||
| Event::EnrAdded { enr, replaced } => {} | ||
| Event::NodeInserted { node_id, replaced } => {} |
There was a problem hiding this comment.
here we need to handle the discovered entries
see
reth/crates/net/network/src/discovery.rs
Line 169 in 32bf97d
| let discv5 = Discv5Handle::convert_to_discv5(discv5); | ||
| let discv5_guard = discv5.lock(); | ||
| let mut local_enr = discv5_guard.local_enr(); | ||
| // local_enr.set_ip(socket_address.ip(),); #5576 figure out how to get signing |
There was a problem hiding this comment.
trying to figure out how to get the signing key to update the ip
|
Some errors need to be handled apart from open comments. |
|
@mattsse any inputs on how to fix the test error. It occurred after using parking lot mutex . |
crates/net/discv5/src/lib.rs
Outdated
| // Wrapper struct for Discv5 | ||
| #[derive(Clone)] | ||
| pub struct Discv5Handle { | ||
| inner: Arc<Mutex<Discv5>>, |
There was a problem hiding this comment.
why do we need a mutex here?
crates/net/discv5/src/lib.rs
Outdated
| self.inner.clone() | ||
| } | ||
|
|
||
| pub async fn start_service(&self) -> Result<(), Discv5Error> { |
There was a problem hiding this comment.
can this not call start directly on the wrapped discv5?
crates/net/network/src/discovery.rs
Outdated
| if let Some(discv5) = &self.discv5 { | ||
| let discv5 = Discv5Handle::convert_to_discv5(discv5); | ||
| let discv5_guard = discv5.lock(); | ||
| let node_id = talk_request.node_id(); | ||
| let enr = discv5_guard.find_enr(&node_id); | ||
| let protocol = talk_request.protocol(); | ||
| let request = talk_request.body(); | ||
| let _ = | ||
| discv5_guard.talk_req(enr.unwrap(), protocol.to_vec(), request.to_vec()); |
There was a problem hiding this comment.
I don't think we need this
|
@supernovahs anything else you'd like to add? else @mattsse bump |
|
closing since #6151 is now the main PR for this |
#1383
TODO
Add relevant traits in https://github.com/sigp/discv5 through a PR