fix(p2p,docs): handle pair-peer peer accounting

docs/xdc/admin/admin.md

@@ -241,6 +241,10 @@ curl -s -X POST -H "Content-Type: application/json" ${RPC} -d '{
 
 The `peers` administrative property can be queried for all the information known about the connected remote nodes at the networking granularity.
 
+The result is an array containing at most one entry per unique remote NodeID.
+If the client temporarily holds multiple physical connections to the same
+remote NodeID, `admin_peers` reports that remote node once.
+
 Parameters:
 
 None

docs/xdc/net/net.md

@@ -37,7 +37,11 @@ Response:
 
 ## Method net_peerCount
 
-The `peerCount` method returns the number of connected peers.
+The `peerCount` method returns the number of connected remote nodes.
+
+The value is counted by unique node identity. If the client temporarily holds
+multiple physical connections to the same remote NodeID, they are reported as a
+single peer by this method.
 
 Parameters:
 

ethclient/ethclient.go

@@ -112,7 +112,8 @@ func (ec *Client) BlockNumber(ctx context.Context) (uint64, error) {
 	return uint64(result), err
 }
 
-// PeerCount returns the number of p2p peers as reported by the net_peerCount method.
+// PeerCount returns the number of connected remote nodes as reported by
+// the net_peerCount method.
 func (ec *Client) PeerCount(ctx context.Context) (uint64, error) {
 	var result hexutil.Uint64
 	err := ec.c.CallContext(ctx, &result, "net_peerCount")

internal/ethapi/api.go

@@ -2416,7 +2416,7 @@ func (s *NetAPI) Listening() bool {
 	return true // always listening
 }
 
-// PeerCount returns the number of connected peers
+// PeerCount returns the number of connected remote nodes.
 func (s *NetAPI) PeerCount() hexutil.Uint {
 	return hexutil.Uint(s.net.PeerCount())
 }

p2p/server.go

@@ -277,7 +277,10 @@ func (c *conn) set(f connFlag, val bool) {
 	atomic.StoreInt32((*int32)(&c.flags), int32(flags))
 }
 
-// Peers returns all connected peers.
+// Peers returns the public view of connected remote nodes.
+//
+// The returned slice contains one entry per remote NodeID, so multiple physical
+// connections associated with the same node are represented by a single entry.
 func (srv *Server) Peers() []*Peer {
 	var ps []*Peer
 	select {
@@ -295,7 +298,11 @@ func (srv *Server) Peers() []*Peer {
 	return ps
 }
 
-// PeerCount returns the number of connected peers.
+// PeerCount returns the number of connected remote nodes.
+//
+// Multiple physical connections associated with the same remote NodeID
+// (for example pair peers) are counted once because the public peer view is
+// keyed by NodeID.
 func (srv *Server) PeerCount() int {
 	var count int
 	select {
@@ -582,6 +589,7 @@ func (srv *Server) run(dialstate dialer) {
 	defer srv.loopWG.Done()
 	var (
 		peers        = make(map[discover.NodeID]*Peer)
+		connCount    = 0
 		inboundCount = 0
 		trusted      = make(map[discover.NodeID]bool, len(srv.TrustedNodes))
 		taskdone     = make(chan task, maxActiveDialTasks)
@@ -702,14 +710,15 @@ running:
 					p.events = &srv.peerFeed
 				}
 				name := truncateName(c.name)
+				connCount++
 
 				go srv.runPeer(p)
 				if peers[c.id] != nil {
 					peers[c.id].PairPeer = p
-					srv.log.Debug("Adding p2p pair peer", "name", name, "addr", c.fd.RemoteAddr(), "peers", len(peers)+1)
+					srv.log.Debug("Adding p2p pair peer", "name", name, "addr", c.fd.RemoteAddr(), "connections", connCount)
 				} else {
 					peers[c.id] = p
-					srv.log.Debug("Adding p2p peer", "name", name, "addr", c.fd.RemoteAddr(), "peers", len(peers)+1)
+					srv.log.Debug("Adding p2p peer", "name", name, "addr", c.fd.RemoteAddr(), "connections", connCount)
 				}
 				if p.Inbound() {
 					inboundCount++
@@ -730,8 +739,8 @@ running:
 		case pd := <-srv.delpeer:
 			// A peer disconnected.
 			d := common.PrettyDuration(mclock.Now() - pd.created)
-			pd.log.Debug("Removing p2p peer", "duration", d, "peers", len(peers)-1, "req", pd.requested, "err", pd.err)
-			delete(peers, pd.ID())
+			connCount = removePeerTracking(peers, pd, connCount)
+			pd.log.Debug("Removing p2p peer", "duration", d, "connections", connCount, "req", pd.requested, "err", pd.err)
 			if pd.Inbound() {
 				inboundCount--
 			}
@@ -755,11 +764,23 @@ running:
 	// Wait for peers to shut down. Pending connections and tasks are
 	// not handled here and will terminate soon-ish because srv.quit
 	// is closed.
-	for len(peers) > 0 {
-		p := <-srv.delpeer
-		p.log.Trace("<-delpeer (spindown)", "remainingTasks", len(runningTasks))
-		delete(peers, p.ID())
+	for connCount > 0 {
+		pd := <-srv.delpeer
+		pd.log.Trace("<-delpeer (spindown)", "remainingTasks", len(runningTasks))
+		connCount = removePeerTracking(peers, pd, connCount)
+	}
+}
+
+func removePeerTracking(peers map[discover.NodeID]*Peer, pd peerDrop, connCount int) int {
+	if connCount > 0 {
+		connCount--
+	}
+	if current := peers[pd.ID()]; current == pd.Peer {
+		delete(peers, pd.ID())
+	} else if current != nil && current.PairPeer == pd.Peer {
+		current.PairPeer = nil
 	}
+	return connCount
 }
 
 func (srv *Server) protoHandshakeChecks(peers map[discover.NodeID]*Peer, inboundCount int, c *conn) error {

p2p/server_test.go

@@ -486,6 +486,26 @@ func TestServerPeerLimits(t *testing.T) {
 	conn.Close()
 }
 
+func TestRemovePeerTrackingKeepsPrimaryOnPairDrop(t *testing.T) {
+	id := randomID()
+	primary := newPeer(&conn{id: id}, nil)
+	pair := newPeer(&conn{id: id}, nil)
+	primary.PairPeer = pair
+
+	peers := map[discover.NodeID]*Peer{id: primary}
+	connCount := removePeerTracking(peers, peerDrop{Peer: pair}, 2)
+
+	if connCount != 1 {
+		t.Fatalf("unexpected connection count: got %d want %d", connCount, 1)
+	}
+	if peers[id] != primary {
+		t.Fatal("primary peer was removed while dropping pair peer")
+	}
+	if primary.PairPeer != nil {
+		t.Fatal("primary peer still references dropped pair peer")
+	}
+}
+
 func TestServerSetupConn(t *testing.T) {
 	id := randomID()
 	srvkey := newkey()