feat(net): network scaffolding #110
Conversation
onbjerg
added
C-enhancement
Codecov Report
@@ Coverage Diff @@
## main #110 +/- ##
==========================================
- Coverage 74.84% 70.88% -3.97%
==========================================
Files 171 186 +15
Lines 14093 15130 +1037
==========================================
+ Hits 10548 10725 +177
- Misses 3545 4405 +860
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
wip? |
|
currently integrating the p2p stream, then this is a ready mvp |
mattsse
force-pushed
the
matt/network-scaffolding
branch
from
d8f9988
to
23dcfa6
Compare
| pub struct Capability { | ||
| /// Name of the Capability | ||
| pub name: SmolStr, | ||
| /// The version of the capability | ||
| pub version: u64, | ||
| } |
There was a problem hiding this comment.
This should probably be unified with what we have in eth-wire/ CapabilityMessage, maybe good for a follow up
There was a problem hiding this comment.
definitely. will be first follow up.
| pub enum CapabilityMessage { | ||
| /// Eth sub-protocol message. | ||
| Eth(EthMessage), | ||
| /// Any other capability message. | ||
| Other(RawCapabilityMessage), | ||
| } |
There was a problem hiding this comment.
Do we need an implementation of Stream/Sink which outputs / takes as input this type?
The current implementation is nice because EthStream can be layered on top of a Stream<Item = Result<Bytes>> but having a way to get this typed CapabilityMessage from a P2PStream would be useful as well
There was a problem hiding this comment.
ideally deserialization will be handled by the session task, so we don't need to do the work in the main network loop and can instead deal directly with the message variants.
| direction: Direction, | ||
| ) { | ||
| let stream = match direction { | ||
| Direction::Incoming => match ECIESStream::incoming(stream, secret_key).await { | ||
| Ok(stream) => stream, | ||
| Err(error) => { | ||
| let _ = events | ||
| .send(PendingSessionEvent::EciesAuthError { remote_addr, session_id, error }) | ||
| .await; | ||
| return | ||
| } | ||
| }, | ||
| Direction::Outgoing(remote_node_id) => { |
| Poll::Ready(Err(_)) => { | ||
| trace!( | ||
| ?id, | ||
| target = "net", | ||
| "Request canceled, response channel closed." | ||
| ); | ||
| disconnect_sessions.push(*id); | ||
| } | ||
| Poll::Pending => continue, |
There was a problem hiding this comment.
Should peer sessions return an error when a remote peer sends a Disconnect message? Or should there be an event type / listener for disconnect notifications instead?
There was a problem hiding this comment.
this is handled in the session manager, which should listen for the disconnect message from the spawned session.
| /// Propagates Block to peers. | ||
| pub(crate) fn announce_block(&mut self, _hash: H256, _block: ()) { | ||
| // TODO propagate the newblock messages to all connected peers that haven't seen the block | ||
| // yet | ||
|
|
||
| todo!() | ||
| } |
There was a problem hiding this comment.
Block gossip is removed in EIP-3675 but other chains that use the devp2p stack may still use this. So we may need to implement this in a configurable way.
Other chains sometimes modify the networking layer in subtle ways, for example BSC's eth/67 is different than ETH's eth/67 (and there are more subprotocols / sync strategies), so we may need to revisit things if we decide to support other chains.
sketch out
Networktype, incoming tcp listener and udp discovery.Blocked by #113