-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Browser relay support #168
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.
Looks good to me, thanks for the summary and the reference links!
Commented a few nitpicks
host/config.go
Outdated
@@ -17,6 +17,7 @@ var defaultConfig = Config{ | |||
Websocket: false, | |||
BootNodesReachabilityCheckInterval: 1 * time.Minute, | |||
MustReachBootNodes: defaultMustReachBootNodes, | |||
EnableP2PRelay: true, |
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.
Interesting - do we want nodes to be relays by default? @dmikey
Potentially we could say only head nodes are relays by default, and in main.go say host.WithEnableP2PRelay(role == blockless.HeadNode)
.
However I do think having more relays would make the network more decentralized and resilient, if there's no drawbacks. I'm cool with the current option of having it on, just bringing it up for discussion.
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 think relays make sense. Let's start with head nodes, and maybe also dialback
flag as a required option when enabling relay. More likely the operator would have tested / knows about inbound routing which would still be needed for the relay to be effective. This will fix our problem with formation dialing as well "could not reach peer" we've been seeing in the wild.
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.
addressed p2p relay default option: 8ebb63b
unsure about which dialback config we are talking about here though.
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.
there is an optional --dialback-address --dialback-port, these are used for advertising the explicit return address to the node, though I don't think I like this after thinking it through. we'll keep it independent.
Context
This feature allows worker node peer-to-peer discovery (especially required for browser nodes) by using b7s node as circuit relayer.
Why is this needed?
Browser sessions cannot directly connect with each other; a relay (or intermediary server) is required first to establish a connection between independent browser sessions.
The connection between the browser nodes needs to occur through a relayer - in this case b7s head node; this can be illustrated in the diagram below:
This update essentially allows the b7s head node to act as a circuit relay for browser nodes; which makes it possible for browser nodes to discover and connect (or dial to) each other over webrtc after the initial connection through the relay (b7s head node in this case) has been established.