Skip to content

Commit 7642729

Browse files
committed
fix(p2p,docs): handle pair-peer peer accounting
Track pair-peer disconnects with a separate connection count so removal logs and shutdown bookkeeping do not underflow or drop the primary peer entry. Clarify that PeerCount, Peers, net_peerCount, and admin_peers are reported by unique remote NodeID rather than physical connection count.
1 parent f5fe86c commit 7642729

6 files changed

Lines changed: 63 additions & 13 deletions

File tree

docs/xdc/admin/admin.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,10 @@ curl -s -X POST -H "Content-Type: application/json" ${RPC} -d '{
241241

242242
The `peers` administrative property can be queried for all the information known about the connected remote nodes at the networking granularity.
243243

244+
The result is an array containing at most one entry per unique remote NodeID.
245+
If the client temporarily holds multiple physical connections to the same
246+
remote NodeID, `admin_peers` reports that remote node once.
247+
244248
Parameters:
245249

246250
None

docs/xdc/net/net.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@ Response:
3737

3838
## Method net_peerCount
3939

40-
The `peerCount` method returns the number of connected peers.
40+
The `peerCount` method returns the number of connected remote nodes.
41+
42+
The value is counted by unique node identity. If the client temporarily holds
43+
multiple physical connections to the same remote NodeID, they are reported as a
44+
single peer by this method.
4145

4246
Parameters:
4347

ethclient/ethclient.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ func (ec *Client) BlockNumber(ctx context.Context) (uint64, error) {
112112
return uint64(result), err
113113
}
114114

115-
// PeerCount returns the number of p2p peers as reported by the net_peerCount method.
115+
// PeerCount returns the number of connected remote nodes as reported by
116+
// the net_peerCount method.
116117
func (ec *Client) PeerCount(ctx context.Context) (uint64, error) {
117118
var result hexutil.Uint64
118119
err := ec.c.CallContext(ctx, &result, "net_peerCount")

internal/ethapi/api.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2416,7 +2416,7 @@ func (s *NetAPI) Listening() bool {
24162416
return true // always listening
24172417
}
24182418

2419-
// PeerCount returns the number of connected peers
2419+
// PeerCount returns the number of connected remote nodes.
24202420
func (s *NetAPI) PeerCount() hexutil.Uint {
24212421
return hexutil.Uint(s.net.PeerCount())
24222422
}

p2p/server.go

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,10 @@ func (c *conn) set(f connFlag, val bool) {
277277
atomic.StoreInt32((*int32)(&c.flags), int32(flags))
278278
}
279279

280-
// Peers returns all connected peers.
280+
// Peers returns the public view of connected remote nodes.
281+
//
282+
// The returned slice contains one entry per remote NodeID, so multiple physical
283+
// connections associated with the same node are represented by a single entry.
281284
func (srv *Server) Peers() []*Peer {
282285
var ps []*Peer
283286
select {
@@ -295,7 +298,11 @@ func (srv *Server) Peers() []*Peer {
295298
return ps
296299
}
297300

298-
// PeerCount returns the number of connected peers.
301+
// PeerCount returns the number of connected remote nodes.
302+
//
303+
// Multiple physical connections associated with the same remote NodeID
304+
// (for example pair peers) are counted once because the public peer view is
305+
// keyed by NodeID.
299306
func (srv *Server) PeerCount() int {
300307
var count int
301308
select {
@@ -582,6 +589,7 @@ func (srv *Server) run(dialstate dialer) {
582589
defer srv.loopWG.Done()
583590
var (
584591
peers = make(map[discover.NodeID]*Peer)
592+
connCount = 0
585593
inboundCount = 0
586594
trusted = make(map[discover.NodeID]bool, len(srv.TrustedNodes))
587595
taskdone = make(chan task, maxActiveDialTasks)
@@ -702,14 +710,15 @@ running:
702710
p.events = &srv.peerFeed
703711
}
704712
name := truncateName(c.name)
713+
connCount++
705714

706715
go srv.runPeer(p)
707716
if peers[c.id] != nil {
708717
peers[c.id].PairPeer = p
709-
srv.log.Debug("Adding p2p pair peer", "name", name, "addr", c.fd.RemoteAddr(), "peers", len(peers)+1)
718+
srv.log.Debug("Adding p2p pair peer", "name", name, "addr", c.fd.RemoteAddr(), "connections", connCount)
710719
} else {
711720
peers[c.id] = p
712-
srv.log.Debug("Adding p2p peer", "name", name, "addr", c.fd.RemoteAddr(), "peers", len(peers)+1)
721+
srv.log.Debug("Adding p2p peer", "name", name, "addr", c.fd.RemoteAddr(), "connections", connCount)
713722
}
714723
if p.Inbound() {
715724
inboundCount++
@@ -730,8 +739,8 @@ running:
730739
case pd := <-srv.delpeer:
731740
// A peer disconnected.
732741
d := common.PrettyDuration(mclock.Now() - pd.created)
733-
pd.log.Debug("Removing p2p peer", "duration", d, "peers", len(peers)-1, "req", pd.requested, "err", pd.err)
734-
delete(peers, pd.ID())
742+
connCount = removePeerTracking(peers, pd, connCount)
743+
pd.log.Debug("Removing p2p peer", "duration", d, "connections", connCount, "req", pd.requested, "err", pd.err)
735744
if pd.Inbound() {
736745
inboundCount--
737746
}
@@ -755,11 +764,23 @@ running:
755764
// Wait for peers to shut down. Pending connections and tasks are
756765
// not handled here and will terminate soon-ish because srv.quit
757766
// is closed.
758-
for len(peers) > 0 {
759-
p := <-srv.delpeer
760-
p.log.Trace("<-delpeer (spindown)", "remainingTasks", len(runningTasks))
761-
delete(peers, p.ID())
767+
for connCount > 0 {
768+
pd := <-srv.delpeer
769+
pd.log.Trace("<-delpeer (spindown)", "remainingTasks", len(runningTasks))
770+
connCount = removePeerTracking(peers, pd, connCount)
771+
}
772+
}
773+
774+
func removePeerTracking(peers map[discover.NodeID]*Peer, pd peerDrop, connCount int) int {
775+
if connCount > 0 {
776+
connCount--
777+
}
778+
if current := peers[pd.ID()]; current == pd.Peer {
779+
delete(peers, pd.ID())
780+
} else if current != nil && current.PairPeer == pd.Peer {
781+
current.PairPeer = nil
762782
}
783+
return connCount
763784
}
764785

765786
func (srv *Server) protoHandshakeChecks(peers map[discover.NodeID]*Peer, inboundCount int, c *conn) error {

p2p/server_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,26 @@ func TestServerPeerLimits(t *testing.T) {
486486
conn.Close()
487487
}
488488

489+
func TestRemovePeerTrackingKeepsPrimaryOnPairDrop(t *testing.T) {
490+
id := randomID()
491+
primary := newPeer(&conn{id: id}, nil)
492+
pair := newPeer(&conn{id: id}, nil)
493+
primary.PairPeer = pair
494+
495+
peers := map[discover.NodeID]*Peer{id: primary}
496+
connCount := removePeerTracking(peers, peerDrop{Peer: pair}, 2)
497+
498+
if connCount != 1 {
499+
t.Fatalf("unexpected connection count: got %d want %d", connCount, 1)
500+
}
501+
if peers[id] != primary {
502+
t.Fatal("primary peer was removed while dropping pair peer")
503+
}
504+
if primary.PairPeer != nil {
505+
t.Fatal("primary peer still references dropped pair peer")
506+
}
507+
}
508+
489509
func TestServerSetupConn(t *testing.T) {
490510
id := randomID()
491511
srvkey := newkey()

0 commit comments

Comments
 (0)