p2p/enode: fix endpoint determination for IPv6 (#29801)

enode.Node has separate accessor functions for getting the IP, UDP port and TCP port.
These methods performed separate checks for attributes set in the ENR.

With this PR, the accessor methods will now return cached information, and the endpoint is
determined when the node is created. The logic to determine the preferred endpoint is now
more correct, and considers how 'global' each address is when both IPv4 and IPv6 addresses
are present in the ENR.

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,76 @@ func New(validSchemes enr.IdentityScheme, r *enr.Record) (*Node, error) {
 	if err := r.VerifySignature(validSchemes); err != nil {
 		return nil, err
 	}
-	node := &Node{r: *r}
+	var id ID
-	if n := copy(node.id[:], validSchemes.NodeAddr(&node.r)); n != len(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 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.IsMulticast()
+}
+
+func localityScore(ip netip.Addr) int {
+	switch {
+	case ip.IsUnspecified():
+		return 0
+	case ip.IsLoopback():
+		return 1
+	case ip.IsLinkLocalUnicast():
+		return 2
+	case ip.IsPrivate():
+		return 3
+	default:
+		return 4
+	}
+}
+
+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 +159,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 (
+	return net.IP(n.ip.AsSlice())
-		ip4 enr.IPv4
+}
-		ip6 enr.IPv6
+
-	)
+// IPAddr returns the IP address of the node.
-	if n.Load(&ip4) == nil {
+func (n *Node) IPAddr() netip.Addr {
-		return net.IP(ip4)
+	return n.ip
-	}
-	if n.Load(&ip6) == nil {
-		return net.IP(ip6)
-	}
-	return nil
 }
 
 // UDP returns the UDP port of the node.
 func (n *Node) UDP() int {
-	var port enr.UDP
+	return int(n.udp)
-	n.Load(&port)
-	return int(port)
 }
 
 // TCP returns the TCP port of the node.
 func (n *Node) TCP() int {
-	var port enr.TCP
+	return int(n.tcp)
-	n.Load(&port)
+}
-	return int(port)
+
+// UDPEndpoint returns the announced TCP endpoint.
+func (n *Node) UDPEndpoint() (netip.AddrPort, bool) {
+	if !n.ip.IsValid() || n.ip.IsUnspecified() || 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.ip.IsUnspecified() || 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 +219,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 {
+	if n.ip.IsMulticast() || n.ip.IsUnspecified() {
-		return errors.New("missing UDP port")
-	}
-	ip := n.IP()
-	if ip.IsMulticast() || 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,167 @@ 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-unspecified",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.IPv4Addr(netip.MustParseAddr("0.0.0.0")))
+				return SignNull(&r, id)
+			}(),
+			wantIP: netip.MustParseAddr("0.0.0.0"),
+		},
+		{
+			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-unspecified-and-ipv6-loopback",
+			node: func() *Node {
+				var r enr.Record
+				r.Set(enr.IPv4Addr(netip.MustParseAddr("0.0.0.0")))
+				r.Set(enr.IPv6Addr(netip.MustParseAddr("::1")))
+				return SignNull(&r, id)
+			}(),
+			wantIP: netip.MustParseAddr("::1"),
+		},
+		{
+			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)
+	var r enr.Record
-	if err := rlp.DecodeBytes(data, &node.r); err != nil {
+	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.
+	if len(id) != len(ID{}) {
-	copy(node.id[:], id)
+		panic(fmt.Errorf("invalid id length %d", len(id)))
-	return node
+	}
+	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()}

p2p/enr/entries.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io"
 	"net"
+	"net/netip"
 
 	"github.com/ethereum/go-ethereum/rlp"
 )
@@ -167,6 +168,60 @@ func (v *IPv6) DecodeRLP(s *rlp.Stream) error {
 	return nil
 }
 
+// IPv4Addr is the "ip" key, which holds the IP address of the node.
+type IPv4Addr netip.Addr
+
+func (v IPv4Addr) ENRKey() string { return "ip" }
+
+// EncodeRLP implements rlp.Encoder.
+func (v IPv4Addr) EncodeRLP(w io.Writer) error {
+	addr := netip.Addr(v)
+	if !addr.Is4() {
+		return fmt.Errorf("address is not IPv4")
+	}
+	enc := rlp.NewEncoderBuffer(w)
+	bytes := addr.As4()
+	enc.WriteBytes(bytes[:])
+	return enc.Flush()
+}
+
+// DecodeRLP implements rlp.Decoder.
+func (v *IPv4Addr) DecodeRLP(s *rlp.Stream) error {
+	var bytes [4]byte
+	if err := s.ReadBytes(bytes[:]); err != nil {
+		return err
+	}
+	*v = IPv4Addr(netip.AddrFrom4(bytes))
+	return nil
+}
+
+// IPv6Addr is the "ip6" key, which holds the IP address of the node.
+type IPv6Addr netip.Addr
+
+func (v IPv6Addr) ENRKey() string { return "ip6" }
+
+// EncodeRLP implements rlp.Encoder.
+func (v IPv6Addr) EncodeRLP(w io.Writer) error {
+	addr := netip.Addr(v)
+	if !addr.Is6() {
+		return fmt.Errorf("address is not IPv6")
+	}
+	enc := rlp.NewEncoderBuffer(w)
+	bytes := addr.As16()
+	enc.WriteBytes(bytes[:])
+	return enc.Flush()
+}
+
+// DecodeRLP implements rlp.Decoder.
+func (v *IPv6Addr) DecodeRLP(s *rlp.Stream) error {
+	var bytes [16]byte
+	if err := s.ReadBytes(bytes[:]); err != nil {
+		return err
+	}
+	*v = IPv6Addr(netip.AddrFrom16(bytes))
+	return nil
+}
+
 // KeyError is an error related to a key.
 type KeyError struct {
 	Key string