feat: quic,webtransport: enable both quic-draft29 and quic-v1 addrs on quic. only quic-v1 on webtransport (#1881)

* transport.Listener  returns a list of multiaddrs

* Support both QUIC versions in QUIC transport

* Support only QUIC v1 in webtransport

* Update dialMatcher

* Update tests

* Only use draft 29 when dialing if the server is a draft 29 server

* Removes QUIC draft 29 addrs if we have a QUIC v1 addr

* Lint fix

* Add changes to deterministic certhashes after rebase

* Update p2p/transport/quic/options.go

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

* Update p2p/transport/quic/listener.go

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

* Update p2p/transport/quic/quic_multiaddr.go

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

* Stylize QUIC correctly

* Update doc around ListenClose

* Preallocate a bit extra to avoid paying for an allocation later

* Keep a list of multiaddrs, then join

* PR nits

* Close transport or listener just once

* Update go-multiaddr

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

Author: MarcoPolo

core/transport/transport.go

@@ -91,7 +91,7 @@ type Listener interface {
 	Accept() (CapableConn, error)
 	Close() error
 	Addr() net.Addr
-	Multiaddr() ma.Multiaddr
+	Multiaddrs() []ma.Multiaddr
 }
 
 // TransportNetwork is an inet.Network with methods for managing transports.

go.mod

@@ -38,7 +38,7 @@ require (
 	github.com/minio/sha256-simd v1.0.0
 	github.com/mr-tron/base58 v1.2.0
 	github.com/multiformats/go-base32 v0.1.0
-	github.com/multiformats/go-multiaddr v0.7.0
+	github.com/multiformats/go-multiaddr v0.8.0
 	github.com/multiformats/go-multiaddr-dns v0.3.1
 	github.com/multiformats/go-multiaddr-fmt v0.1.0
 	github.com/multiformats/go-multibase v0.1.1

go.sum

@@ -373,8 +373,8 @@ github.com/multiformats/go-base36 v0.1.0 h1:JR6TyF7JjGd3m6FbLU2cOxhC0Li8z8dLNGQ8
 github.com/multiformats/go-base36 v0.1.0/go.mod h1:kFGE83c6s80PklsHO9sRn2NCoffoRdUUOENyW/Vv6sM=
 github.com/multiformats/go-multiaddr v0.1.1/go.mod h1:aMKBKNEYmzmDmxfX88/vz+J5IU55txyt0p4aiWVohjo=
 github.com/multiformats/go-multiaddr v0.2.0/go.mod h1:0nO36NvPpyV4QzvTLi/lafl2y95ncPj0vFwVF6k6wJ4=
-github.com/multiformats/go-multiaddr v0.7.0 h1:gskHcdaCyPtp9XskVwtvEeQOG465sCohbQIirSyqxrc=
-github.com/multiformats/go-multiaddr v0.7.0/go.mod h1:Fs50eBDWvZu+l3/9S6xAE7ZYj6yhxlvaVZjakWN7xRs=
+github.com/multiformats/go-multiaddr v0.8.0 h1:aqjksEcqK+iD/Foe1RRFsGZh8+XFiGo7FgUCZlpv3LU=
+github.com/multiformats/go-multiaddr v0.8.0/go.mod h1:Fs50eBDWvZu+l3/9S6xAE7ZYj6yhxlvaVZjakWN7xRs=
 github.com/multiformats/go-multiaddr-dns v0.3.1 h1:QgQgR+LQVt3NPTjbrLLpsaT2ufAA2y0Mkk+QRVJbW3A=
 github.com/multiformats/go-multiaddr-dns v0.3.1/go.mod h1:G/245BRQ6FJGmryJCrOuTdB37AMA5AMOVuO6NY3JwTk=
 github.com/multiformats/go-multiaddr-fmt v0.1.0 h1:WLEFClPycPkp4fnIzoFoV9FVd49/eQsuaL3/CWe167E=

p2p/host/autonat/dialpolicy_test.go

@@ -47,9 +47,9 @@ func (l *mockL) Accept() (transport.CapableConn, error) {
 	<-l.ctx.Done()
 	return nil, errors.New("expected in mocked test")
 }
-func (l *mockL) Close() error                   { return nil }
-func (l *mockL) Addr() net.Addr                 { return nil }
-func (l *mockL) Multiaddr() multiaddr.Multiaddr { return l.addr }
+func (l *mockL) Close() error                      { return nil }
+func (l *mockL) Addr() net.Addr                    { return nil }
+func (l *mockL) Multiaddrs() []multiaddr.Multiaddr { return []multiaddr.Multiaddr{l.addr} }
 
 func TestSkipDial(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())

p2p/net/swarm/swarm.go

@@ -237,8 +237,14 @@ func (s *Swarm) close() {
 	s.transports.m = nil
 	s.transports.Unlock()
 
-	var wg sync.WaitGroup
+	// Dedup transports that may be listening on multiple protocols
+	transportsToClose := make(map[transport.Transport]struct{}, len(transports))
 	for _, t := range transports {
+		transportsToClose[t] = struct{}{}
+	}
+
+	var wg sync.WaitGroup
+	for t := range transportsToClose {
 		if closer, ok := t.(io.Closer); ok {
 			wg.Add(1)
 			go func(c io.Closer) {

p2p/net/swarm/swarm_addr.go

@@ -16,9 +16,9 @@ func (s *Swarm) ListenAddresses() []ma.Multiaddr {
 }
 
 func (s *Swarm) listenAddressesNoLock() []ma.Multiaddr {
-	addrs := make([]ma.Multiaddr, 0, len(s.listeners.m))
+	addrs := make([]ma.Multiaddr, 0, len(s.listeners.m)+10) // A bit extra so we may avoid an extra allocation in the for loop below.
 	for l := range s.listeners.m {
-		addrs = append(addrs, l.Multiaddr())
+		addrs = append(addrs, l.Multiaddrs()...)
 	}
 	return addrs
 }

p2p/net/swarm/swarm_addr_test.go

@@ -98,7 +98,7 @@ func TestDialAddressSelection(t *testing.T) {
 	require.Equal(t, tcpTr, s.TransportForDialing(ma.StringCast("/ip4/127.0.0.1/tcp/1234")))
 	require.Equal(t, quicTr, s.TransportForDialing(ma.StringCast("/ip4/127.0.0.1/udp/1234/quic")))
 	require.Equal(t, circuitTr, s.TransportForDialing(ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic/p2p-circuit/p2p/%s", id))))
-	require.Equal(t, webtransportTr, s.TransportForDialing(ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic/webtransport/certhash/%s", certHash))))
+	require.Equal(t, webtransportTr, s.TransportForDialing(ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic-v1/webtransport/certhash/%s", certHash))))
 	require.Nil(t, s.TransportForDialing(ma.StringCast("/ip4/1.2.3.4")))
 	require.Nil(t, s.TransportForDialing(ma.StringCast("/ip4/1.2.3.4/tcp/443/ws")))
 }

p2p/net/swarm/swarm_dial.go

@@ -12,6 +12,7 @@ import (
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/libp2p/go-libp2p/core/peerstore"
 	"github.com/libp2p/go-libp2p/core/transport"
+	"github.com/lucas-clemente/quic-go"
 
 	ma "github.com/multiformats/go-multiaddr"
 	madns "github.com/multiformats/go-multiaddr-dns"
@@ -439,15 +440,17 @@ func (s *Swarm) filterKnownUndialables(p peer.ID, addrs []ma.Multiaddr) []ma.Mul
 		}
 	}
 
-	return maybeRemoveWebTransportAddrs(ma.FilterAddrs(addrs,
-		func(addr ma.Multiaddr) bool { return !ma.Contains(ourAddrs, addr) },
-		s.canDial,
-		// TODO: Consider allowing link-local addresses
-		func(addr ma.Multiaddr) bool { return !manet.IsIP6LinkLocal(addr) },
-		func(addr ma.Multiaddr) bool {
-			return s.gater == nil || s.gater.InterceptAddrDial(p, addr)
-		},
-	))
+	return maybeRemoveWebTransportAddrs(
+		maybeRemoveQUICDraft29(
+			ma.FilterAddrs(addrs,
+				func(addr ma.Multiaddr) bool { return !ma.Contains(ourAddrs, addr) },
+				s.canDial,
+				// TODO: Consider allowing link-local addresses
+				func(addr ma.Multiaddr) bool { return !manet.IsIP6LinkLocal(addr) },
+				func(addr ma.Multiaddr) bool {
+					return s.gater == nil || s.gater.InterceptAddrDial(p, addr)
+				},
+			)))
 }
 
 // limitedDial will start a dial to the given peer when
@@ -536,9 +539,32 @@ func isWebTransport(addr ma.Multiaddr) bool {
 	return err == nil
 }
 
-func isQUIC(addr ma.Multiaddr) bool {
-	_, err := addr.ValueForProtocol(ma.P_QUIC)
-	return err == nil && !isWebTransport(addr)
+func quicVersion(addr ma.Multiaddr) (quic.VersionNumber, bool) {
+	found := false
+	foundWebTransport := false
+	var version quic.VersionNumber
+	ma.ForEach(addr, func(c ma.Component) bool {
+		switch c.Protocol().Code {
+		case ma.P_QUIC:
+			version = quic.VersionDraft29
+			found = true
+			return true
+		case ma.P_QUIC_V1:
+			version = quic.Version1
+			found = true
+			return true
+		case ma.P_WEBTRANSPORT:
+			version = quic.Version1
+			foundWebTransport = true
+			return false
+		default:
+			return true
+		}
+	})
+	if foundWebTransport {
+		return 0, false
+	}
+	return version, found
 }
 
 // If we have QUIC addresses, we don't want to dial WebTransport addresses.
@@ -548,7 +574,7 @@ func isQUIC(addr ma.Multiaddr) bool {
 func maybeRemoveWebTransportAddrs(addrs []ma.Multiaddr) []ma.Multiaddr {
 	var hasQuic, hasWebTransport bool
 	for _, addr := range addrs {
-		if isQUIC(addr) {
+		if _, isQuic := quicVersion(addr); isQuic {
 			hasQuic = true
 		}
 		if isWebTransport(addr) {
@@ -568,3 +594,38 @@ func maybeRemoveWebTransportAddrs(addrs []ma.Multiaddr) []ma.Multiaddr {
 	}
 	return addrs[:c]
 }
+
+// If we have QUIC V1 addresses, we don't want to dial QUIC draft29 addresses.
+// This is a similar hack to the above. If we add one more hack like this, let's
+// define a `Filterer` interface like the `Resolver` interface that transports
+// can optionally implement if they want to filter the multiaddrs.
+//
+// This mutates the input
+func maybeRemoveQUICDraft29(addrs []ma.Multiaddr) []ma.Multiaddr {
+	var hasQuicV1, hasQuicDraft29 bool
+	for _, addr := range addrs {
+		v, isQuic := quicVersion(addr)
+		if !isQuic {
+			continue
+		}
+
+		if v == quic.Version1 {
+			hasQuicV1 = true
+		}
+		if v == quic.VersionDraft29 {
+			hasQuicDraft29 = true
+		}
+	}
+	if !hasQuicDraft29 || !hasQuicV1 {
+		return addrs
+	}
+	var c int
+	for _, addr := range addrs {
+		if v, isQuic := quicVersion(addr); isQuic && v == quic.VersionDraft29 {
+			continue
+		}
+		addrs[c] = addr
+		c++
+	}
+	return addrs[:c]
+}

p2p/net/swarm/swarm_dial_test.go

@@ -196,11 +196,23 @@ func TestAddrResolutionRecursive(t *testing.T) {
 func TestRemoveWebTransportAddrs(t *testing.T) {
 	tcpAddr := ma.StringCast("/ip4/9.5.6.4/tcp/1234")
 	quicAddr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic")
-	webtransportAddr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic/webtransport")
+	webtransportAddr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic-v1/webtransport")
 
 	require.Equal(t, []ma.Multiaddr{tcpAddr, quicAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{tcpAddr, quicAddr}))
 	require.Equal(t, []ma.Multiaddr{tcpAddr, webtransportAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{tcpAddr, webtransportAddr}))
 	require.Equal(t, []ma.Multiaddr{tcpAddr, quicAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{tcpAddr, webtransportAddr, quicAddr}))
 	require.Equal(t, []ma.Multiaddr{quicAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{quicAddr, webtransportAddr}))
 	require.Equal(t, []ma.Multiaddr{webtransportAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{webtransportAddr}))
 }
+
+func TestRemoveQuicDraft29(t *testing.T) {
+	tcpAddr := ma.StringCast("/ip4/9.5.6.4/tcp/1234")
+	quicDraft29Addr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic")
+	quicV1Addr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic-v1")
+
+	require.Equal(t, []ma.Multiaddr{tcpAddr, quicV1Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{tcpAddr, quicV1Addr}))
+	require.Equal(t, []ma.Multiaddr{tcpAddr, quicDraft29Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{tcpAddr, quicDraft29Addr}))
+	require.Equal(t, []ma.Multiaddr{tcpAddr, quicV1Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{tcpAddr, quicDraft29Addr, quicV1Addr}))
+	require.Equal(t, []ma.Multiaddr{quicV1Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{quicV1Addr, quicDraft29Addr}))
+	require.Equal(t, []ma.Multiaddr{quicDraft29Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{quicDraft29Addr}))
+}

p2p/net/swarm/swarm_listen.go

@@ -37,23 +37,27 @@ func (s *Swarm) Listen(addrs ...ma.Multiaddr) error {
 	return nil
 }
 
-// ListenClose stop and delete listeners for all of the given addresses.
+// ListenClose stop and delete listeners for all of the given addresses. If an
+// any address belongs to one of the addreses a Listener provides, then the
+// Listener will close for *all* addresses it provides. For example if you close
+// and address with `/quic`, then the QUIC listener will close and also close
+// any `/quic-v1` address.
 func (s *Swarm) ListenClose(addrs ...ma.Multiaddr) {
-	var listenersToClose []transport.Listener
+	listenersToClose := make(map[transport.Listener]struct{}, len(addrs))
 
 	s.listeners.Lock()
 	for l := range s.listeners.m {
-		if !containsMultiaddr(addrs, l.Multiaddr()) {
+		if !containsSomeMultiaddr(addrs, l.Multiaddrs()) {
 			continue
 		}
 
 		delete(s.listeners.m, l)
-		listenersToClose = append(listenersToClose, l)
+		listenersToClose[l] = struct{}{}
 	}
 	s.listeners.cacheEOL = time.Time{}
 	s.listeners.Unlock()
 
-	for _, l := range listenersToClose {
+	for l := range listenersToClose {
 		l.Close()
 	}
 }
@@ -92,11 +96,13 @@ func (s *Swarm) AddListenAddr(a ma.Multiaddr) error {
 	s.listeners.cacheEOL = time.Time{}
 	s.listeners.Unlock()
 
-	maddr := list.Multiaddr()
+	maddrs := list.Multiaddrs()
 
 	// signal to our notifiees on listen.
 	s.notifyAll(func(n network.Notifiee) {
-		n.Listen(s, maddr)
+		for _, maddr := range maddrs {
+			n.Listen(s, maddr)
+		}
 	})
 
 	go func() {
@@ -116,7 +122,9 @@ func (s *Swarm) AddListenAddr(a ma.Multiaddr) error {
 
 			// signal to our notifiees on listen close.
 			s.notifyAll(func(n network.Notifiee) {
-				n.ListenClose(s, maddr)
+				for _, maddr := range maddrs {
+					n.ListenClose(s, maddr)
+				}
 			})
 			s.refs.Done()
 		}()
@@ -147,11 +155,16 @@ func (s *Swarm) AddListenAddr(a ma.Multiaddr) error {
 	return nil
 }
 
-func containsMultiaddr(addrs []ma.Multiaddr, addr ma.Multiaddr) bool {
-	for _, a := range addrs {
-		if addr == a {
+func containsSomeMultiaddr(hayStack []ma.Multiaddr, needles []ma.Multiaddr) bool {
+	seenSet := make(map[string]struct{}, len(needles))
+	for _, a := range needles {
+		seenSet[string(a.Bytes())] = struct{}{}
+	}
+	for _, a := range hayStack {
+		if _, found := seenSet[string(a.Bytes())]; found {
 			return true
 		}
 	}
 	return false
+
 }