chore(p2p): prefer tcp when discovering bootnodes

pkg/p2p/discover.go

@@ -9,11 +9,19 @@ import (
 	"errors"
 	"fmt"
 	"math/rand"
+	"sort"
 
+	"github.com/ethersphere/bee/v2/pkg/bzz"
 	ma "github.com/multiformats/go-multiaddr"
 	madns "github.com/multiformats/go-multiaddr-dns"
 )
 
+func sortAddrsByTCPPreference(addrs []ma.Multiaddr) {
+	sort.SliceStable(addrs, func(i, j int) bool {
+		return bzz.ClassifyTransport(addrs[i]).Priority() < bzz.ClassifyTransport(addrs[j]).Priority()
+	})
+}
+
 func isDNSProtocol(protoCode int) bool {
 	if protoCode == ma.P_DNS || protoCode == ma.P_DNS4 || protoCode == ma.P_DNS6 || protoCode == ma.P_DNSADDR {
 		return true
@@ -35,9 +43,15 @@ func Discover(ctx context.Context, addr ma.Multiaddr, f func(ma.Multiaddr) (bool
 		return false, errors.New("non-resolvable API endpoint")
 	}
 
-	rand.Shuffle(len(addrs), func(i, j int) {
-		addrs[i], addrs[j] = addrs[j], addrs[i]
-	})
+	// If the resolved addresses are real (non-DNS) multiaddrs, order them
+	// with TCP first. Otherwise (still DNS), shuffle randomly as before.
+	if comp, _ := ma.SplitFirst(addrs[0]); !isDNSProtocol(comp.Protocol().Code) {
+		sortAddrsByTCPPreference(addrs)
+	} else {
+		rand.Shuffle(len(addrs), func(i, j int) {
+			addrs[i], addrs[j] = addrs[j], addrs[i]
+		})
+	}
 	for _, addr := range addrs {
 		stopped, err := Discover(ctx, addr, f)
 		if err != nil {

pkg/p2p/discover_test.go

@@ -0,0 +1,145 @@
+// Copyright 2024 The Swarm Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p2p_test
+
+import (
+	"context"
+	"testing"
+
+	ma "github.com/multiformats/go-multiaddr"
+
+	"github.com/ethersphere/bee/v2/pkg/p2p"
+)
+
+// TestTCPPreferenceOrdering verifies that sortAddrsByTCPPreference places TCP
+// addresses before non-TCP addresses regardless of input order.
+func TestTCPPreferenceOrdering(t *testing.T) {
+	mustAddr := func(s string) ma.Multiaddr {
+		a, err := ma.NewMultiaddr(s)
+		if err != nil {
+			t.Fatalf("parse multiaddr %q: %v", s, err)
+		}
+		return a
+	}
+
+	hasTCP := func(a ma.Multiaddr) bool {
+		for _, p := range a.Protocols() {
+			if p.Code == ma.P_TCP {
+				return true
+			}
+		}
+		return false
+	}
+
+	for _, tc := range []struct {
+		name  string
+		input []string
+	}{
+		{
+			name: "quic before tcp",
+			input: []string{
+				"/ip4/1.2.3.4/udp/1234/quic-v1",
+				"/ip4/1.2.3.4/tcp/1234",
+			},
+		},
+		{
+			name: "mixed quic tcp ws",
+			input: []string{
+				"/ip4/1.2.3.4/udp/1234/quic-v1",
+				"/ip4/1.2.3.4/tcp/1234/ws",
+				"/ip4/1.2.3.4/tcp/1234",
+			},
+		},
+		{
+			name: "all non-tcp",
+			input: []string{
+				"/ip4/1.2.3.4/udp/1234/quic-v1",
+				"/ip4/5.6.7.8/udp/5678/quic-v1",
+			},
+		},
+		{
+			name: "all tcp",
+			input: []string{
+				"/ip4/1.2.3.4/tcp/1234",
+				"/ip4/5.6.7.8/tcp/5678",
+			},
+		},
+		{
+			// Real addresses from _dnsaddr.bee-0.testnet.ethswarm.org, placed
+			// with TLS/WS first so the sort must move the plain TCP entry ahead.
+			name: "real testnet bee-0 tls before tcp",
+			input: []string{
+				"/ip4/49.12.172.37/tcp/32550/tls/sni/49-12-172-37.k2k4r8norg8tz5giqfwkcqyrbbtyoikcxaioqjwckz6lgbvhbs7yswy5.libp2p.direct/ws/p2p/QmZsYCbkUXWpfR34PmUwMJvHwJtGfbcMMoAp1G2EydkpRA",
+				"/ip4/49.12.172.37/tcp/32490/p2p/QmZsYCbkUXWpfR34PmUwMJvHwJtGfbcMMoAp1G2EydkpRA",
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			addrs := make([]ma.Multiaddr, len(tc.input))
+			for i, s := range tc.input {
+				addrs[i] = mustAddr(s)
+			}
+
+			p2p.SortAddrsByTCPPreference(addrs)
+
+			// Once we've seen a non-TCP address, no TCP address may follow it.
+			seenNonTCP := false
+			for _, a := range addrs {
+				if !hasTCP(a) {
+					seenNonTCP = true
+				} else if seenNonTCP {
+					t.Errorf("TCP address %s appears after a non-TCP address", a)
+				}
+			}
+		})
+	}
+}
+
+// TestDiscoverDNS performs a real DNS resolution of the testnet and mainnet
+// bootnodes using p2p.Discover with the default DNS resolver.
+func TestDiscoverDNS(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		bootnode string
+	}{
+		{
+			name:     "testnet",
+			bootnode: "/dnsaddr/testnet.ethswarm.org",
+		},
+		{
+			name:     "mainnet",
+			bootnode: "/dnsaddr/mainnet.ethswarm.org",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			addr, err := ma.NewMultiaddr(tc.bootnode)
+			if err != nil {
+				t.Fatalf("parse multiaddr %q: %v", tc.bootnode, err)
+			}
+
+			var resolved []ma.Multiaddr
+			_, err = p2p.Discover(context.Background(), addr, func(a ma.Multiaddr) (bool, error) {
+				resolved = append(resolved, a)
+				return false, nil
+			})
+			if err != nil {
+				t.Fatalf("Discover(%q): %v", tc.bootnode, err)
+			}
+
+			if len(resolved) == 0 {
+				t.Fatalf("Discover(%q): resolved no addresses", tc.bootnode)
+			}
+
+			t.Logf("resolved %d address(es) for %s:", len(resolved), tc.name)
+			for _, a := range resolved {
+				t.Logf("  %s", a)
+			}
+		})
+	}
+}

pkg/p2p/export_test.go

@@ -0,0 +1,7 @@
+// Copyright 2024 The Swarm Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p2p
+
+var SortAddrsByTCPPreference = sortAddrsByTCPPreference

pkg/topology/kademlia/kademlia.go

@@ -829,9 +829,13 @@ func (k *Kad) connectBootNodes(ctx context.Context) {
 			k.metrics.TotalBootNodesConnectionAttempts.Inc()
 
 			if err != nil {
+				if errors.Is(err, p2p.ErrUnsupportedAddresses) {
+					k.logger.Debug("bootnode address transport not supported, skipping", "bootnode_address", addr)
+					return false, nil
+				}
 				if !errors.Is(err, p2p.ErrAlreadyConnected) {
 					k.logger.Error(err, "connect to bootnode failed", "bootnode_address", addr)
-					return false, err
+					return false, nil
 				}
 				k.logger.Debug("bootnode already connected", "bootnode_address", addr)
 				return false, nil