From c8bd6b5711c2346828fc103325191e9985c04f59 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 5 Jul 2022 01:33:14 -0700 Subject: [PATCH] fix: return the best _acceptable_ conn in NewStream (#1604) * fix: return the best _acceptable_ conn in NewStream Otherwise, we can return, e.g., a transient connection that can't actually be used. * fix: fail dial if we can't have a usable connection If we have a transient connection, don't want to use a transient connection, and haven't specified "force direct dial", fail the dial. --- p2p/net/swarm/dial_worker.go | 8 ++++---- p2p/net/swarm/swarm.go | 31 +++++++++++++++++++++++-------- p2p/net/swarm/swarm_dial.go | 9 +++++---- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/p2p/net/swarm/dial_worker.go b/p2p/net/swarm/dial_worker.go index 8d5d43419..cf9dfe592 100644 --- a/p2p/net/swarm/dial_worker.go +++ b/p2p/net/swarm/dial_worker.go @@ -89,9 +89,9 @@ loop: return } - c := w.s.bestAcceptableConnToPeer(req.ctx, w.peer) - if c != nil { - req.resch <- dialResponse{conn: c} + c, err := w.s.bestAcceptableConnToPeer(req.ctx, w.peer) + if c != nil || err != nil { + req.resch <- dialResponse{conn: c, err: err} continue loop } @@ -255,7 +255,7 @@ func (w *dialWorker) dispatchError(ad *addrDial, err error) { // all addrs have erred, dispatch dial error // but first do a last one check in case an acceptable connection has landed from // a simultaneous dial that started later and added new acceptable addrs - c := w.s.bestAcceptableConnToPeer(pr.req.ctx, w.peer) + c, _ := w.s.bestAcceptableConnToPeer(pr.req.ctx, w.peer) if c != nil { pr.req.resch <- dialResponse{conn: c} } else { diff --git a/p2p/net/swarm/swarm.go b/p2p/net/swarm/swarm.go index e14a068c7..532ae2445 100644 --- a/p2p/net/swarm/swarm.go +++ b/p2p/net/swarm/swarm.go @@ -350,7 +350,11 @@ func (s *Swarm) NewStream(ctx context.Context, p peer.ID) (network.Stream, error dials := 0 for { // will prefer direct connections over relayed connections for opening streams - c := s.bestConnToPeer(p) + c, err := s.bestAcceptableConnToPeer(ctx, p) + if err != nil { + return nil, err + } + if c == nil { if nodial, _ := network.GetNoDial(ctx); nodial { return nil, network.ErrNoConn @@ -447,15 +451,26 @@ func (s *Swarm) bestConnToPeer(p peer.ID) *Conn { return best } -func (s *Swarm) bestAcceptableConnToPeer(ctx context.Context, p peer.ID) *Conn { +// - Returns the best "acceptable" connection, if available. +// - Returns nothing if no such connection exists, but if we should try dialing anyways. +// - Returns an error if no such connection exists, but we should not try dialing. +func (s *Swarm) bestAcceptableConnToPeer(ctx context.Context, p peer.ID) (*Conn, error) { conn := s.bestConnToPeer(p) - if conn != nil { - forceDirect, _ := network.GetForceDirectDial(ctx) - if !forceDirect || isDirectConn(conn) { - return conn - } + if conn == nil { + return nil, nil } - return nil + + forceDirect, _ := network.GetForceDirectDial(ctx) + if forceDirect && !isDirectConn(conn) { + return nil, nil + } + + useTransient, _ := network.GetUseTransient(ctx) + if useTransient || !conn.Stat().Transient { + return conn, nil + } + + return nil, network.ErrTransientConn } func isDirectConn(c *Conn) bool { diff --git a/p2p/net/swarm/swarm_dial.go b/p2p/net/swarm/swarm_dial.go index d2562a143..f8c0e0e68 100644 --- a/p2p/net/swarm/swarm_dial.go +++ b/p2p/net/swarm/swarm_dial.go @@ -244,10 +244,11 @@ func (s *Swarm) dialPeer(ctx context.Context, p peer.ID) (*Conn, error) { return nil, ErrDialToSelf } - // check if we already have an open (usable) connection first - conn := s.bestAcceptableConnToPeer(ctx, p) - if conn != nil { - return conn, nil + // check if we already have an open (usable) connection first, or can't have a usable + // connection. + conn, err := s.bestAcceptableConnToPeer(ctx, p) + if conn != nil || err != nil { + return conn, err } if s.gater != nil && !s.gater.InterceptPeerDial(p) {