only initialize the hole puncher once a public address was discovered

p2p/protocol/holepunch/holepuncher.go

@@ -42,8 +42,6 @@ type holePuncher struct {
 
 	ids identify.IDService
 
-	hasPublicAddrsChan <-chan struct{}
-
 	// active hole punches for deduplicating
 	activeMx sync.Mutex
 	active   map[peer.ID]struct{}
@@ -54,13 +52,12 @@ type holePuncher struct {
 	tracer *tracer
 }
 
-func newHolePuncher(h host.Host, ids identify.IDService, hasPublicAddrsChan <-chan struct{}, tracer *tracer) *holePuncher {
+func newHolePuncher(h host.Host, ids identify.IDService, tracer *tracer) *holePuncher {
 	hp := &holePuncher{
-		host:               h,
-		ids:                ids,
-		active:             make(map[peer.ID]struct{}),
-		hasPublicAddrsChan: hasPublicAddrsChan,
-		tracer:             tracer,
+		host:   h,
+		ids:    ids,
+		active: make(map[peer.ID]struct{}),
+		tracer: tracer,
 	}
 	hp.ctx, hp.ctxCancel = context.WithCancel(context.Background())
 	h.Network().Notify((*netNotifiee)(hp))
@@ -133,16 +130,6 @@ func (hp *holePuncher) directConnect(rp peer.ID) error {
 	}
 
 	log.Debugw("got inbound proxy conn", "peer", rp)
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-	defer cancel()
-	select {
-	case <-hp.ctx.Done():
-		return hp.ctx.Err()
-	case <-ctx.Done():
-		log.Debug("didn't find any public host address")
-		return errors.New("can't initiate hole punch, as we don't have any public addresses")
-	case <-hp.hasPublicAddrsChan:
-	}
 
 	// hole punch
 	for i := 0; i < maxRetries; i++ {

p2p/protocol/holepunch/svc.go

@@ -47,11 +47,11 @@ type Service struct {
 	ids         identify.IDService
 	holePuncher *holePuncher
 
+	hasPublicAddrsChan chan struct{}
+
 	tracer *tracer
 
 	refCount sync.WaitGroup
-
-	hasPublicAddrsChan chan struct{} // this chan is closed as soon as we have a public address
 }
 
 // NewService creates a new service that can be used for hole punching
@@ -65,7 +65,7 @@ func NewService(h host.Host, ids identify.IDService, opts ...Option) (*Service,
 	}
 
 	ctx, cancel := context.WithCancel(context.Background())
-	hs := &Service{
+	s := &Service{
 		ctx:                ctx,
 		ctxCancel:          cancel,
 		host:               h,
@@ -74,17 +74,16 @@ func NewService(h host.Host, ids identify.IDService, opts ...Option) (*Service,
 	}
 
 	for _, opt := range opts {
-		if err := opt(hs); err != nil {
+		if err := opt(s); err != nil {
 			cancel()
 			return nil, err
 		}
 	}
-	hs.holePuncher = newHolePuncher(h, ids, hs.hasPublicAddrsChan, hs.tracer)
 
-	hs.refCount.Add(1)
-	go hs.watchForPublicAddr()
+	s.refCount.Add(1)
+	go s.watchForPublicAddr()
 
-	return hs, nil
+	return s, nil
 }
 
 func (s *Service) watchForPublicAddr() {
@@ -104,6 +103,7 @@ func (s *Service) watchForPublicAddr() {
 		if containsPublicAddr(s.ids.OwnObservedAddrs()) {
 			log.Debug("Host now has a public address. Starting holepunch protocol.")
 			s.host.SetStreamHandler(Protocol, s.handleNewStream)
+			s.holePuncher = newHolePuncher(s.host, s.ids, s.tracer)
 			close(s.hasPublicAddrsChan)
 			return
 		}
@@ -232,5 +232,6 @@ func (s *Service) handleNewStream(str network.Stream) {
 // DirectConnect is only exposed for testing purposes.
 // TODO: find a solution for this.
 func (s *Service) DirectConnect(p peer.ID) error {
+	<-s.hasPublicAddrsChan
 	return s.holePuncher.DirectConnect(p)
 }