test: Not redialing connections if in pending inbound state

Author: the-shadow-coder

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.
 

p2p/server_test.go

@@ -463,6 +463,109 @@ func TestServerSetupConn(t *testing.T) {
 	}
 }
 
+// TestServerPendingInboundRejection checks that duplicate inbound connections
+// are rejected when there's already a pending inbound connection from the same peer.
+func TestServerPendingInboundRejection(t *testing.T) {
+	trustedNode := newkey()
+	srv := &Server{
+		Config: Config{
+			PrivateKey:  newkey(),
+			MaxPeers:    10,
+			NoDial:      true,
+			NoDiscovery: true,
+			Logger:      testlog.Logger(t, log.LvlTrace),
+		},
+	}
+	if err := srv.Start(); err != nil {
+		t.Fatalf("could not start: %v", err)
+	}
+	defer srv.Stop()
+
+	newconn := func(id enode.ID) *conn {
+		fd, _ := net.Pipe()
+		tx := newTestTransport(&trustedNode.PublicKey, fd, nil)
+		node := enode.SignNull(new(enr.Record), id)
+		return &conn{fd: fd, transport: tx, flags: inboundConn, node: node, cont: make(chan error)}
+	}
+
+	// Create two connections from the same peer
+	peerID := randomID()
+	c1 := newconn(peerID)
+	c2 := newconn(peerID)
+
+	// First connection enters pendingInbound
+	err1 := srv.checkpoint(c1, srv.checkpointPostHandshake)
+	if err1 != nil {
+		t.Fatalf("first connection failed unexpectedly: %v", err1)
+	}
+
+	// Second connection should be rejected (duplicate pending inbound)
+	err2 := srv.checkpoint(c2, srv.checkpointPostHandshake)
+	if err2 != DiscAlreadyConnected {
+		t.Errorf("expected DiscAlreadyConnected for duplicate pending inbound, got: %v", err2)
+	}
+
+	t.Logf("✅ First connection accepted, second rejected with: %v", err2)
+}
+
+// TestServerPendingInboundCleanup checks that pending inbound state is properly
+// cleaned up when a connection fails or completes.
+func TestServerPendingInboundCleanup(t *testing.T) {
+	// Track when peers connect
+	connected := make(chan *Peer, 2)
+	remid := &newkey().PublicKey
+
+	srv := startTestServer(t, remid, func(p *Peer) {
+		connected <- p
+	})
+	defer close(connected)
+	defer srv.Stop()
+
+	// First connection attempt - will succeed
+	conn1, err := net.DialTimeout("tcp", srv.ListenAddr, 5*time.Second)
+	if err != nil {
+		t.Fatalf("could not dial: %v", err)
+	}
+	defer conn1.Close()
+
+	// Wait for peer to connect
+	select {
+	case peer := <-connected:
+		t.Logf("First connection succeeded, peer: %s", peer.ID())
+
+		// Disconnect the peer to clean up pendingInbound
+		peer.Disconnect(DiscRequested)
+		time.Sleep(100 * time.Millisecond)
+
+		// Verify peer is gone
+		if srv.PeerCount() != 0 {
+			t.Errorf("expected 0 peers after disconnect, got %d", srv.PeerCount())
+		}
+
+	case <-time.After(2 * time.Second):
+		t.Fatal("first connection did not complete within timeout")
+	}
+
+	// Second connection attempt from same peer - should succeed now
+	// because pendingInbound was cleaned up
+	conn2, err := net.DialTimeout("tcp", srv.ListenAddr, 5*time.Second)
+	if err != nil {
+		t.Fatalf("could not dial second time: %v", err)
+	}
+	defer conn2.Close()
+
+	select {
+	case peer := <-connected:
+		t.Logf("Second connection succeeded after cleanup, peer: %s", peer.ID())
+		if peer.ID() != enode.PubkeyToIDV4(remid) {
+			t.Errorf("peer has wrong id")
+		}
+
+	case <-time.After(2 * time.Second):
+		t.Error("second connection did not complete within timeout - pendingInbound may not have been cleaned up")
+	}
+}
+
 type setupTransport struct {
 	pubkey            *ecdsa.PublicKey
 	encHandshakeErr   error