-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Utility subsystem for actually connecting to network #1205
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this. I also predict a whole bunch of merge conflicts with #1185. Which of us should merge the other PR in to handle that merge conflict work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would each Subsystem have its own Substrate notification protocol, or share one, dispatching via custom message types?
I wrote this with the understanding that all subsystems will share a notification protocol provided by the |
some nits Co-authored-by: Max Inden <[email protected]>
Depends on #1199 .
The idea is to avoid shared ownership of a network service, peer reputation, peer discovery (although I didn't describe discovery here...).
Also it lets us consolidate view change updates in only one place, without worrying about race conditions between different protocols as all network traffic will pass through this subsystem.
One downside is that all network traffic will pass through the overseer, but I assume this will not be too much of an overhead as the messages are light from the overseer's perspective, only a few hundred bytes at most.
sizeof(AllMessages)
would be interesting, though, and we may want to do some internal boxing if that turns out to be a bottleneck, but I expect task-switching to be even more of one.Alternatively I think it would be OK to have all network events passed through a side-channel but I have avoided that possibility for the moment before relaxing any more assumptions.