-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
webrtcprivate: add transport #2576
base: master
Are you sure you want to change the base?
Changes from all commits
3a54181
8f84284
4f3c12e
5ab60e2
10624d1
0b20ffe
072b38a
8bf790f
4622ab2
1448286
6e464e5
8baee51
255fd3e
f41c97c
a2e3df3
212159b
7e077d3
13081dd
05f5d33
4a77bae
6249772
bc90626
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -598,3 +598,11 @@ func SwarmOpts(opts ...swarm.Option) Option { | |
return nil | ||
} | ||
} | ||
|
||
func EnableWebRTCPrivate(stunServers []config.ICEServer) Option { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this a separate option? We already have |
||
return func(cfg *Config) error { | ||
cfg.WebRTCPrivate = true | ||
cfg.WebRTCStunServers = stunServers | ||
return nil | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,7 +27,10 @@ func (s *Swarm) TransportForDialing(a ma.Multiaddr) transport.Transport { | |
} | ||
return nil | ||
} | ||
if isRelayAddr(a) { | ||
if isProtocolAddr(a, ma.P_WEBRTC) { | ||
return s.transports.m[ma.P_WEBRTC] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this require extra logic here? Is it because of the certhash? Why doesn't it apply to WebTransport then? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The peer address is a circuitv2 address. So we need to check it before the circuitv2 check but the check is the same as the circuit v2 check. The address is of the form There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes but we don't want the circuit-v2 transport to handle dialing to the webrtc address.
But now we need
This is because the webrtcprivate address has a p2p-circuit component but we want to use webrtcprivate transport. I could have written this as:
The flow I've implemented is:
I've chosen this because it is similar to circuitv2 transport which first establishes a direct connection with the relay node and then a circuitv2 connection to the peer over the relayed connection. This also has the nice property that Transport.Dial can connect to the peer over the address no matter the state of the host when Transport.Dial is called. In this case, it means if no circuit-v2 connection exists, it'll make a circuit-v2 connection first. |
||
} | ||
if isProtocolAddr(a, ma.P_CIRCUIT) { | ||
return s.transports.m[ma.P_CIRCUIT] | ||
} | ||
for _, t := range s.transports.m { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,6 +84,8 @@ func NewService(h host.Host, ids identify.IDService, opts ...Option) (*Service, | |
return nil, err | ||
} | ||
} | ||
s.host.Network().Notify(s) | ||
|
||
s.tracer.Start() | ||
|
||
s.refCount.Add(1) | ||
|
@@ -283,3 +285,42 @@ func (s *Service) DirectConnect(p peer.ID) error { | |
s.holePuncherMx.Unlock() | ||
return holePuncher.DirectConnect(p) | ||
} | ||
|
||
var _ network.Notifiee = &Service{} | ||
|
||
func (s *Service) Connected(_ network.Network, conn network.Conn) { | ||
// Dial /webrtc address if it's a relay connection to a browser node | ||
if conn.Stat().Direction == network.DirOutbound && conn.Stat().Transient { | ||
s.refCount.Add(1) | ||
go func() { | ||
defer s.refCount.Done() | ||
select { | ||
// waiting for Identify here will allow us to access the peer's public and observed addresses | ||
// that we can dial to for a hole punch. | ||
case <-s.ids.IdentifyWait(conn): | ||
case <-s.ctx.Done(): | ||
return | ||
} | ||
p := conn.RemotePeer() | ||
// Peer supports DCUtR, let it trigger holepunch | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if the peer supports DCUtR, but there are only WebRTC addresses? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That case is handled here: https://github.com/libp2p/go-libp2p/blob/webrtcprivate/transport/p2p/protocol/holepunch/holepuncher.go#L142 In this case the peer(DCUtR initiator) is best suited to handle the call so I've kept it on that side. |
||
if protos, err := s.host.Peerstore().SupportsProtocols(p, Protocol); err == nil && len(protos) > 0 { | ||
return | ||
} | ||
// No DCUtR support, connect with peer over /webrtc | ||
for _, addr := range s.host.Peerstore().Addrs(p) { | ||
if _, err := addr.ValueForProtocol(ma.P_WEBRTC); err == nil { | ||
ctx := network.WithForceDirectDial(s.ctx, "webrtc holepunch") | ||
err := s.host.Connect(ctx, peer.AddrInfo{ID: p}) // address is already in peerstore | ||
if err != nil { | ||
log.Debugf("holepunch attempt to %s over /webrtc failed: %s", p, err) | ||
} | ||
return | ||
} | ||
} | ||
}() | ||
} | ||
} | ||
|
||
func (*Service) Disconnected(_ network.Network, v network.Conn) {} | ||
func (*Service) Listen(n network.Network, a ma.Multiaddr) {} | ||
func (*Service) ListenClose(n network.Network, a ma.Multiaddr) {} |
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.
Why do we need to redeclare this? Is it a requirement for Fx?
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.
Nope. Just so that we redefine it a different way later if we want. But I think it's fine to remove this and just take the pion specific object. We can make a breaking change if this needs to be changed.
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.
It’s a breaking change anyway if we change the underlying type. That should be fine.
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.
Can you remove it?