Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Utility subsystem for actually connecting to network #1205

Merged
merged 8 commits into from
Jun 10, 2020

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Jun 5, 2020

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.

@rphmeier rphmeier added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes labels Jun 5, 2020
Copy link
Contributor

@coriolinus coriolinus left a 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?

@rphmeier
Copy link
Contributor Author

rphmeier commented Jun 9, 2020

Once #1185 is merged I'll merge it into #1199 , and then it shouldn't be too hard to rebase this on top of that again.

Copy link
Contributor

@mxinden mxinden left a 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?

roadmap/implementors-guide/guide.md Outdated Show resolved Hide resolved
roadmap/implementors-guide/guide.md Outdated Show resolved Hide resolved
roadmap/implementors-guide/guide.md Outdated Show resolved Hide resolved
@rphmeier
Copy link
Contributor Author

rphmeier commented Jun 9, 2020

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 NetworkBridge. And that they will communicate with the NetworkBridge via the overseer.

@rphmeier rphmeier added A8-mergeoncegreen and removed A0-please_review Pull request needs code review. labels Jun 10, 2020
@rphmeier rphmeier merged commit 5629242 into master Jun 10, 2020
@rphmeier rphmeier deleted the rh-networking-architecture branch June 10, 2020 17:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
B0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants