Dispatch uTP listener events to overlay protocol #337
Conversation
ogenev
force-pushed
the
dispatch-utp-listener-events
branch
from
fdfdba5 to
23faa61
Compare
ogenev
changed the title
ogenev
force-pushed
the
dispatch-utp-listener-events
branch
3 times, most recently
from
17816eb to
c98ae9d
Compare
ogenev
force-pushed
the
dispatch-utp-listener-events
branch
from
c98ae9d to
5f28053
Compare
| history_network_task, | ||
| history_event_tx, | ||
| history_utp_tx, | ||
| history_jsonrpc_tx, |
There was a problem hiding this comment.
Not so much a comment to fix in this pr, but just thinking out loud that it would be nice if we come up with a nice abstraction to wrap up all these channels in. With the header oracle in #331, we'll have yet another object to pass around. And when considering the other overlay networks to be implemented, this pattern seems like it's only going to continue to get messier.
There was a problem hiding this comment.
Yeah, maybe we should open an issue and try to refactor this at some point...
trin-history/src/events.rs
Outdated
| loop { | ||
| tokio::select! { | ||
| Some(talk_request) = self.event_rx.recv() => { | ||
| let reply = match self |
There was a problem hiding this comment.
nit shouldn't this block be indented once more
There was a problem hiding this comment.
Seems like cargo fmt doesn't know how to handle select! properly? The other select was funky too.
One way to avoid the issue is by doing:
Some(talk_request) = self.event_rx.recv() => self.handle_history_talk_request(talk_request),
Apparently not, or CI should have complained. |
trin-core/src/portalnet/events.rs
Outdated
| Some(event) = self.protocol_receiver.recv() => { | ||
| let request = match event { |
There was a problem hiding this comment.
Another spot where it seems like there ought to be an indent. Weird that fmt isn't catching these...
| Some(event) = self.protocol_receiver.recv() => { | |
| let request = match event { | |
| Some(event) = self.protocol_receiver.recv() => { | |
| let request = match event { |
| /// Main loop to dispatch `Discv5` and `UtpListener` events to overlay networks | ||
| pub async fn start(mut self) { | ||
| loop { | ||
| tokio::select! { |
There was a problem hiding this comment.
This select is huge. I expect it to be much easier to read if it calls some functions here at each of the branches. Even some of the branches seem large enough to break down into a couple functions.
I don't feel confident about having reviewed this section until that happens.
There was a problem hiding this comment.
Refactored this a bit, it should be more readable now. It seems extracting the select! logic into smaller methods solves also the ident issues.
| @@ -174,6 +174,15 @@ impl<TContentKey: OverlayContentKey + Send, TMetric: Metric + Send> | |||
| self.send_overlay_request(request, direction).await | |||
| } | |||
|
|
|||
| /// Process overlay uTP listener event | |||
| pub fn process_utp_event(&self, event: UtpListenerEvent) { | |||
| // TODO: Handle overlay uTP events | |||
There was a problem hiding this comment.
Normally I would say to move this into an issue. But if you already have the PR up in draft form, I am fine leaving an in-code todo comment. 👍🏻
trin-history/src/events.rs
Outdated
| loop { | ||
| tokio::select! { | ||
| Some(talk_request) = self.event_rx.recv() => { | ||
| let reply = match self |
There was a problem hiding this comment.
Seems like cargo fmt doesn't know how to handle select! properly? The other select was funky too.
One way to avoid the issue is by doing:
Some(talk_request) = self.event_rx.recv() => self.handle_history_talk_request(talk_request),
ogenev
force-pushed
the
dispatch-utp-listener-events
branch
from
5f28053 to
4b50f97
Compare
Subtask of #313.
What was wrong?
In a previous PR, we implemented an uTP listener architecture that allows us to emit global uTP listener events when a uTP stream is closed.
To be able to process those events in the overlay layer, we need a way to handle them.
How was it fixed?
The current design decision is to handle the uTP listener events in the global portal event handler and dispatch them to the overlay network handlers.
To-Do