p2p/enode: fix endpoint determination for IPv6

p2p/enode/idscheme.go

@@ -157,5 +157,5 @@ func SignNull(r *enr.Record, id ID) *Node {
 	if err := r.SetSig(NullID{}, []byte{}); err != nil {
 		panic(err)
 	}
-	return &Node{r: *r, id: id}
+	return newNodeWithID(r, id)
 }

p2p/enode/node.go

@@ -24,6 +24,7 @@ import (
 	"fmt"
 	"math/bits"
 	"net"
+	"net/netip"
 	"strings"
 
 	"github.com/ethereum/go-ethereum/p2p/enr"
@@ -36,6 +37,10 @@ var errMissingPrefix = errors.New("missing 'enr:' prefix for base64-encoded reco
 type Node struct {
 	r  enr.Record
 	id ID
+	// endpoint information
+	ip  netip.Addr
+	udp uint16
+	tcp uint16
 }
 
 // New wraps a node record. The record must be valid according to the given
@@ -44,11 +49,74 @@ func New(validSchemes enr.IdentityScheme, r *enr.Record) (*Node, error) {
 	if err := r.VerifySignature(validSchemes); err != nil {
 		return nil, err
 	}
-	node := &Node{r: *r}
-	if n := copy(node.id[:], validSchemes.NodeAddr(&node.r)); n != len(ID{}) {
-		return nil, fmt.Errorf("invalid node ID length %d, need %d", n, len(ID{}))
+	var id ID
+	if n := copy(id[:], validSchemes.NodeAddr(r)); n != len(id) {
+		return nil, fmt.Errorf("invalid node ID length %d, need %d", n, len(id))
+	}
+	return newNodeWithID(r, id), nil
+}
+
+func newNodeWithID(r *enr.Record, id ID) *Node {
+	n := &Node{r: *r, id: id}
+	// Set the preferred endpoint.
+	// Here we decide between IPv4 and IPv6, choosing the 'most global' address.
+	var ip4 netip.Addr
+	var ip6 netip.Addr
+	n.Load((*enr.IPv4Addr)(&ip4))
+	n.Load((*enr.IPv6Addr)(&ip6))
+	valid4 := validIP(ip4)
+	valid6 := validIP(ip6)
+	switch {
+	case valid4 && valid6:
+		if localityScore(ip4) >= localityScore(ip6) {
+			n.setIP4(ip4)
+		} else {
+			n.setIP6(ip6)
+		}
+	case valid4:
+		n.setIP4(ip4)
+	case valid6:
+		n.setIP6(ip6)
+	}
+	return n
+}
+
+// validIP reports whether 'ip' is a valid node endpoint IP address.
+func validIP(ip netip.Addr) bool {
+	return ip.IsValid() && !ip.IsUnspecified() && !ip.IsMulticast()
+}
+
+func localityScore(ip netip.Addr) int {
+	switch {
+	case ip.IsLoopback():
+		return 0
+	case ip.IsLinkLocalUnicast():
+		return 1
+	case ip.IsPrivate():
+		return 2
+	default:
+		return 3
+	}
+}
+
+func (n *Node) setIP4(ip netip.Addr) {
+	n.ip = ip
+	n.Load((*enr.UDP)(&n.udp))
+	n.Load((*enr.TCP)(&n.tcp))
+}
+
+func (n *Node) setIP6(ip netip.Addr) {
+	if ip.Is4In6() {
+		n.setIP4(ip)
+		return
+	}
+	n.ip = ip
+	if err := n.Load((*enr.UDP6)(&n.udp)); err != nil {
+		n.Load((*enr.UDP)(&n.udp))
+	}
+	if err := n.Load((*enr.TCP6)(&n.tcp)); err != nil {
+		n.Load((*enr.TCP)(&n.tcp))
 	}
-	return node, nil
 }
 
 // MustParse parses a node record or enode:// URL. It panics if the input is invalid.
@@ -89,43 +157,45 @@ func (n *Node) Seq() uint64 {
 	return n.r.Seq()
 }
 
-// Incomplete returns true for nodes with no IP address.
-func (n *Node) Incomplete() bool {
-	return n.IP() == nil
-}
-
 // Load retrieves an entry from the underlying record.
 func (n *Node) Load(k enr.Entry) error {
 	return n.r.Load(k)
 }
 
-// IP returns the IP address of the node. This prefers IPv4 addresses.
+// IP returns the IP address of the node.
 func (n *Node) IP() net.IP {
-	var (
-		ip4 enr.IPv4
-		ip6 enr.IPv6
-	)
-	if n.Load(&ip4) == nil {
-		return net.IP(ip4)
-	}
-	if n.Load(&ip6) == nil {
-		return net.IP(ip6)
-	}
-	return nil
+	return net.IP(n.ip.AsSlice())
+}
+
+// IPAddr returns the IP address of the node.
+func (n *Node) IPAddr() netip.Addr {
+	return n.ip
 }
 
 // UDP returns the UDP port of the node.
 func (n *Node) UDP() int {
-	var port enr.UDP
-	n.Load(&port)
-	return int(port)
+	return int(n.udp)
 }
 
 // TCP returns the TCP port of the node.
 func (n *Node) TCP() int {
-	var port enr.TCP
-	n.Load(&port)
-	return int(port)
+	return int(n.tcp)
+}
+
+// UDPEndpoint returns the announced TCP endpoint.
+func (n *Node) UDPEndpoint() (netip.AddrPort, bool) {
+	if !n.ip.IsValid() || n.udp == 0 {
+		return netip.AddrPort{}, false
+	}
+	return netip.AddrPortFrom(n.ip, n.udp), true
+}
+
+// TCPEndpoint returns the announced TCP endpoint.
+func (n *Node) TCPEndpoint() (netip.AddrPort, bool) {
+	if !n.ip.IsValid() || n.tcp == 0 {
+		return netip.AddrPort{}, false
+	}
+	return netip.AddrPortFrom(n.ip, n.udp), true
 }
 
 // Pubkey returns the secp256k1 public key of the node, if present.
@@ -147,16 +217,15 @@ func (n *Node) Record() *enr.Record {
 // ValidateComplete checks whether n has a valid IP and UDP port.
 // Deprecated: don't use this method.
 func (n *Node) ValidateComplete() error {
-	if n.Incomplete() {
+	if !n.ip.IsValid() {
 		return errors.New("missing IP address")
 	}
-	if n.UDP() == 0 {
-		return errors.New("missing UDP port")
-	}
-	ip := n.IP()
-	if ip.IsMulticast() || ip.IsUnspecified() {
+	if n.ip.IsMulticast() || n.ip.IsUnspecified() {
 		return errors.New("invalid IP (multicast/unspecified)")
 	}
+	if n.udp == 0 {
+		return errors.New("missing UDP port")
+	}
 	// Validate the node key (on curve, etc.).
 	var key Secp256k1
 	return n.Load(&key)

p2p/enode/node_test.go

@@ -21,6 +21,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"math/big"
+	"net/netip"
 	"testing"
 	"testing/quick"
 
@@ -64,6 +65,148 @@ func TestPythonInterop(t *testing.T) {
 	}
 }
 
+func TestNodeEndpoints(t *testing.T) {
+	id := HexID("00000000000000806ad9b61fa5ae014307ebdc964253adcd9f2c0a392aa11abc")
+	type endpointTest struct {
+		name    string
+		node    *Node
+		wantIP  netip.Addr
+		wantUDP int
+		wantTCP int
+	}
+	tests := []endpointTest{
+		{
+			name: "no-addr",
+			node: func() *Node {
+				var r enr.Record
+				return SignNull(&r, id)
+			}(),
+		},
+		{
+			name: "udp-only",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.UDP(9000))
+				return SignNull(&r, id)
+			}(),
+		},
+		{
+			name: "tcp-only",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.TCP(9000))
+				return SignNull(&r, id)
+			}(),
+		},
+		{
+			name: "ipv4-only-loopback",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.IPv4Addr(netip.MustParseAddr("127.0.0.1")))
+				return SignNull(&r, id)
+			}(),
+			wantIP: netip.MustParseAddr("127.0.0.1"),
+		},
+		{
+			name: "ipv4-only",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.IPv4Addr(netip.MustParseAddr("99.22.33.1")))
+				return SignNull(&r, id)
+			}(),
+			wantIP: netip.MustParseAddr("99.22.33.1"),
+		},
+		{
+			name: "ipv6-only",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329")))
+				return SignNull(&r, id)
+			}(),
+			wantIP: netip.MustParseAddr("2001::ff00:0042:8329"),
+		},
+		{
+			name: "ipv4-loopback-and-ipv6-global",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.IPv4Addr(netip.MustParseAddr("127.0.0.1")))
+				r.Set(enr.UDP(30304))
+				r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329")))
+				r.Set(enr.UDP6(30306))
+				return SignNull(&r, id)
+			}(),
+			wantIP:  netip.MustParseAddr("2001::ff00:0042:8329"),
+			wantUDP: 30306,
+		},
+		{
+			name: "ipv4-private-and-ipv6-global",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.IPv4Addr(netip.MustParseAddr("192.168.2.2")))
+				r.Set(enr.UDP(30304))
+				r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329")))
+				r.Set(enr.UDP6(30306))
+				return SignNull(&r, id)
+			}(),
+			wantIP:  netip.MustParseAddr("2001::ff00:0042:8329"),
+			wantUDP: 30306,
+		},
+		{
+			name: "ipv4-local-and-ipv6-global",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.IPv4Addr(netip.MustParseAddr("169.254.2.6")))
+				r.Set(enr.UDP(30304))
+				r.Set(enr.IPv6Addr(netip.MustParseAddr("2001::ff00:0042:8329")))
+				r.Set(enr.UDP6(30306))
+				return SignNull(&r, id)
+			}(),
+			wantIP:  netip.MustParseAddr("2001::ff00:0042:8329"),
+			wantUDP: 30306,
+		},
+		{
+			name: "ipv4-private-and-ipv6-private",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.IPv4Addr(netip.MustParseAddr("192.168.2.2")))
+				r.Set(enr.UDP(30304))
+				r.Set(enr.IPv6Addr(netip.MustParseAddr("fd00::abcd:1")))
+				r.Set(enr.UDP6(30306))
+				return SignNull(&r, id)
+			}(),
+			wantIP:  netip.MustParseAddr("192.168.2.2"),
+			wantUDP: 30304,
+		},
+		{
+			name: "ipv4-private-and-ipv6-link-local",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.IPv4Addr(netip.MustParseAddr("192.168.2.2")))
+				r.Set(enr.UDP(30304))
+				r.Set(enr.IPv6Addr(netip.MustParseAddr("fe80::1")))
+				r.Set(enr.UDP6(30306))
+				return SignNull(&r, id)
+			}(),
+			wantIP:  netip.MustParseAddr("192.168.2.2"),
+			wantUDP: 30304,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if test.wantIP != test.node.IPAddr() {
+				t.Errorf("node has wrong IP %v, want %v", test.node.IPAddr(), test.wantIP)
+			}
+			if test.wantUDP != test.node.UDP() {
+				t.Errorf("node has wrong UDP port %d, want %d", test.node.UDP(), test.wantUDP)
+			}
+			if test.wantTCP != test.node.TCP() {
+				t.Errorf("node has wrong TCP port %d, want %d", test.node.TCP(), test.wantTCP)
+			}
+		})
+	}
+}
+
 func TestHexID(t *testing.T) {
 	ref := ID{0, 0, 0, 0, 0, 0, 0, 128, 106, 217, 182, 31, 165, 174, 1, 67, 7, 235, 220, 150, 66, 83, 173, 205, 159, 44, 10, 57, 42, 161, 26, 188}
 	id1 := HexID("0x00000000000000806ad9b61fa5ae014307ebdc964253adcd9f2c0a392aa11abc")

p2p/enode/nodedb.go

@@ -26,6 +26,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/ethereum/go-ethereum/p2p/enr"
 	"github.com/ethereum/go-ethereum/rlp"
 	"github.com/syndtr/goleveldb/leveldb"
 	"github.com/syndtr/goleveldb/leveldb/errors"
@@ -242,13 +243,14 @@ func (db *DB) Node(id ID) *Node {
 }
 
 func mustDecodeNode(id, data []byte) *Node {
-	node := new(Node)
-	if err := rlp.DecodeBytes(data, &node.r); err != nil {
+	var r enr.Record
+	if err := rlp.DecodeBytes(data, &r); err != nil {
 		panic(fmt.Errorf("p2p/enode: can't decode node %x in DB: %v", id, err))
 	}
-	// Restore node id cache.
-	copy(node.id[:], id)
-	return node
+	if len(id) != len(ID{}) {
+		panic(fmt.Errorf("invalid id length %d", len(id)))
+	}
+	return newNodeWithID(&r, ID(id))
 }
 
 // UpdateNode inserts - potentially overwriting - a node into the peer database.

p2p/enode/urlv4.go

@@ -181,7 +181,7 @@ func (n *Node) URLv4() string {
 		nodeid = fmt.Sprintf("%s.%x", scheme, n.id[:])
 	}
 	u := url.URL{Scheme: "enode"}
-	if n.Incomplete() {
+	if !n.ip.IsValid() {
 		u.Host = nodeid
 	} else {
 		addr := net.TCPAddr{IP: n.IP(), Port: n.TCP()}