From 4f0847c5191b21f51c2c0ef0cdece0c880e7fae3 Mon Sep 17 00:00:00 2001 From: jeevan-sid Date: Tue, 9 Dec 2025 23:04:59 +0530 Subject: [PATCH] test: Not redialing connections if in pending inbound state and diff cleanup --- p2p/dial_test.go | 76 ++++++++++++++++++++++++++++++++++++++++++++++++ p2p/server.go | 15 ++++------ 2 files changed, 81 insertions(+), 10 deletions(-) diff --git a/p2p/dial_test.go b/p2p/dial_test.go index f18dacce2a..9684aa6e91 100644 --- a/p2p/dial_test.go +++ b/p2p/dial_test.go @@ -423,6 +423,82 @@ func TestDialSchedDNSHostname(t *testing.T) { }) } +// This test checks that nodes with pending inbound connections are not dialed. +func TestDialSchedPendingInbound(t *testing.T) { + t.Parallel() + + config := dialConfig{ + maxActiveDials: 5, + maxDialPeers: 4, + } + runDialTest(t, config, []dialTestRound{ + // 2 peers are connected, leaving 2 dial slots. + // Node 0x03 has a pending inbound connection. + // Discovered nodes 0x03, 0x04, 0x05 but only 0x04 and 0x05 should be dialed. + { + peersAdded: []*conn{ + {flags: dynDialedConn, node: newNode(uintID(0x01), "127.0.0.1:30303")}, + {flags: dynDialedConn, node: newNode(uintID(0x02), "127.0.0.2:30303")}, + }, + update: func(d *dialScheduler) { + d.inboundPending(uintID(0x03)) + }, + discovered: []*enode.Node{ + newNode(uintID(0x03), "127.0.0.3:30303"), // not dialed because pending inbound + newNode(uintID(0x04), "127.0.0.4:30303"), + newNode(uintID(0x05), "127.0.0.5:30303"), + }, + wantNewDials: []*enode.Node{ + newNode(uintID(0x04), "127.0.0.4:30303"), + newNode(uintID(0x05), "127.0.0.5:30303"), + }, + }, + // Pending inbound connection for 0x03 completes successfully. + // Node 0x03 becomes a connected peer. + // One dial slot remains, node 0x06 is dialed. + { + update: func(d *dialScheduler) { + // Pending inbound completes + d.inboundCompleted(uintID(0x03)) + }, + peersAdded: []*conn{ + {flags: inboundConn, node: newNode(uintID(0x03), "127.0.0.3:30303")}, + }, + succeeded: []enode.ID{ + uintID(0x04), + }, + failed: []enode.ID{ + uintID(0x05), + }, + discovered: []*enode.Node{ + newNode(uintID(0x03), "127.0.0.3:30303"), // not dialed, now connected + newNode(uintID(0x06), "127.0.0.6:30303"), + }, + wantNewDials: []*enode.Node{ + newNode(uintID(0x06), "127.0.0.6:30303"), + }, + }, + // Inbound peer 0x03 disconnects. + // Another pending inbound starts for 0x07. + // Only 0x03 should be dialed, not 0x07. + { + peersRemoved: []enode.ID{ + uintID(0x03), + }, + update: func(d *dialScheduler) { + d.inboundPending(uintID(0x07)) + }, + discovered: []*enode.Node{ + newNode(uintID(0x03), "127.0.0.3:30303"), + newNode(uintID(0x07), "127.0.0.7:30303"), // not dialed because pending inbound + }, + wantNewDials: []*enode.Node{ + newNode(uintID(0x03), "127.0.0.3:30303"), + }, + }, + }) +} + // ------- // Code below here is the framework for the tests above. diff --git a/p2p/server.go b/p2p/server.go index 3dc2aed845..6d2323f9ce 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -682,8 +682,7 @@ running: case c := <-srv.checkpointPostHandshake: // A connection has passed the encryption handshake so // the remote identity is known (but hasn't been verified yet). - nodeID := c.node.ID() - if trusted[nodeID] { + if trusted[c.node.ID()] { // Ensure that the trusted flag is set before checking against MaxPeers. c.flags |= trustedConn } @@ -696,14 +695,12 @@ running: case c := <-srv.checkpointAddPeer: // At this point the connection is past the protocol handshake. // Its capabilities are known and the remote identity is verified. - nodeID := c.node.ID() err := srv.addPeerChecks(peers, inboundCount, c) if err == nil { // The handshakes are done and it passed all checks. p := srv.launchPeer(c) - peers[nodeID] = p - srv.log.Debug("Adding p2p peer", "peercount", len(peers), "id", p.ID(), - "conn", c.flags, "addr", p.RemoteAddr(), "name", p.Name()) + peers[c.node.ID()] = p + srv.log.Debug("Adding p2p peer", "peercount", len(peers), "id", p.ID(), "conn", c.flags, "addr", p.RemoteAddr(), "name", p.Name()) srv.dialsched.peerAdded(c) if p.Inbound() { inboundCount++ @@ -720,10 +717,8 @@ running: case pd := <-srv.delpeer: // A peer disconnected. d := common.PrettyDuration(mclock.Now() - pd.created) - nodeID := pd.ID() - delete(peers, nodeID) - srv.log.Debug("Removing p2p peer", "peercount", len(peers), "id", nodeID, - "duration", d, "req", pd.requested, "err", pd.err) + delete(peers, pd.ID()) + srv.log.Debug("Removing p2p peer", "peercount", len(peers), "id", pd.ID(), "duration", d, "req", pd.requested, "err", pd.err) srv.dialsched.peerRemoved(pd.rw) if pd.Inbound() { inboundCount--