server+connmgr: make outbound startup deterministic

In this commit, we define TargetOutbound as the number of automatic
connections and treat explicit Connect requests as additional peers.

Permanent requests could previously consume connection IDs before Start
sampled the shared counter. The resulting automatic count depended on
goroutine scheduling, while the listener reserved the worst-case total.
We now start the configured automatic count directly and cap it by the
peer budget left after permanent peers.

The composition test covers permanent requests on both sides of Start and
pins the same automatic+permanent total for either ordering.

connmgr/connmanager.go

@@ -127,8 +127,9 @@ type Config struct {
 	// behavior, while a pointer to zero disables inbound connections.
 	MaxInbound *uint32
 
-	// TargetOutbound is the number of outbound network connections to
+	// TargetOutbound is the number of automatic outbound network connections
-	// maintain. Defaults to 8.
+	// to maintain. Connections made through Connect are additional. Defaults
+	// to 8.
 	TargetOutbound uint32
 
 	// RetryDuration is the duration to wait before retrying connection
@@ -616,7 +617,7 @@ func (cm *ConnManager) Start() {
 		}
 	}
 
-	for i := atomic.LoadUint64(&cm.connReqCount); i < uint64(cm.cfg.TargetOutbound); i++ {
+	for i := uint32(0); i < cm.cfg.TargetOutbound; i++ {
 		go cm.NewConnReq()
 	}
 }

connmgr/connmanager_test.go

@@ -371,6 +371,114 @@ func TestTargetOutbound(t *testing.T) {
 	cmgr.Stop()
 }
 
+// TestTargetOutboundComposition verifies that explicit permanent connections
+// are additional to the automatic outbound target regardless of whether they
+// receive connection request IDs before or after the manager starts.
+func TestTargetOutboundComposition(t *testing.T) {
+	const (
+		targetOutbound = uint32(3)
+		permanentPeers = uint64(2)
+	)
+
+	tests := []struct {
+		name                 string
+		permanentBeforeStart bool
+	}{
+		{name: "permanent before start", permanentBeforeStart: true},
+		{name: "permanent after start"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			connected := make(chan *ConnReq,
+				int(targetOutbound)+int(permanentPeers))
+			cmgr, err := New(&Config{
+				TargetOutbound: targetOutbound,
+				Dial:           mockDialer,
+				GetNewAddress: func() (net.Addr, error) {
+					return &net.TCPAddr{
+						IP:   net.ParseIP("127.0.0.1"),
+						Port: 18555,
+					}, nil
+				},
+				OnConnection: func(c *ConnReq, _ net.Conn) {
+					connected <- c
+				},
+			})
+			if err != nil {
+				t.Fatalf("New error: %v", err)
+			}
+
+			connectPermanent := func() {
+				for i := uint64(0); i < permanentPeers; i++ {
+					go cmgr.Connect(&ConnReq{
+						Addr: &net.TCPAddr{
+							IP:   net.ParseIP("127.0.0.1"),
+							Port: 18556 + int(i),
+						},
+						Permanent: true,
+					})
+				}
+			}
+
+			if test.permanentBeforeStart {
+				connectPermanent()
+
+				deadline := time.After(time.Second)
+				for atomic.LoadUint64(&cmgr.connReqCount) <
+					permanentPeers {
+
+					select {
+					case <-deadline:
+						t.Fatal("permanent requests did not receive IDs")
+					case <-time.After(time.Millisecond):
+					}
+				}
+			}
+
+			cmgr.Start()
+			if !test.permanentBeforeStart {
+				connectPermanent()
+			}
+
+			var (
+				automaticCount int
+				permanentCount int
+			)
+			for i := 0; i < int(targetOutbound)+int(permanentPeers); i++ {
+				select {
+				case connReq := <-connected:
+					if connReq.Permanent {
+						permanentCount++
+					} else {
+						automaticCount++
+					}
+
+				case <-time.After(time.Second):
+					t.Fatal("timed out waiting for outbound connections")
+				}
+			}
+
+			if automaticCount != int(targetOutbound) {
+				t.Fatalf("unexpected automatic count: got %d, want %d",
+					automaticCount, targetOutbound)
+			}
+			if permanentCount != int(permanentPeers) {
+				t.Fatalf("unexpected permanent count: got %d, want %d",
+					permanentCount, permanentPeers)
+			}
+
+			select {
+			case connReq := <-connected:
+				t.Fatalf("unexpected extra connection: %v", connReq)
+			case <-time.After(10 * time.Millisecond):
+			}
+
+			cmgr.Stop()
+		})
+	}
+}
+
 // TestRetryPermanent tests that permanent connection requests are retried.
 //
 // We make a permanent connection request using Connect, disconnect it using

server.go

@@ -74,6 +74,24 @@ var (
 // zeroHash is the zero value hash (all zeros).  It is defined as a convenience.
 var zeroHash chainhash.Hash
 
+// targetOutboundPeers returns the automatic outbound target for the configured
+// peer mode after permanent peers reserve their portion of the total budget.
+func targetOutboundPeers(
+	maxPeers, permanentPeers int, automaticOutbound bool,
+) int {
+
+	if !automaticOutbound || permanentPeers >= maxPeers {
+		return 0
+	}
+
+	available := maxPeers - permanentPeers
+	if available < defaultTargetOutbound {
+		return available
+	}
+
+	return defaultTargetOutbound
+}
+
 // reservedOutboundPeers returns the outbound connection reservation for the
 // configured peer mode, capped at the total peer limit.
 func reservedOutboundPeers(
@@ -3225,17 +3243,20 @@ func newServer(listenAddrs, agentBlacklist, agentWhitelist []string,
 	}
 
 	// Create a connection manager.
-	targetOutbound := defaultTargetOutbound
-	if cfg.MaxPeers < targetOutbound {
-		targetOutbound = cfg.MaxPeers
-	}
 	permanentPeerCount := len(cfg.ConnectPeers)
 	if permanentPeerCount == 0 {
 		permanentPeerCount = len(cfg.AddPeers)
 	}
+	automaticOutbound := newAddressFunc != nil
+	targetOutbound := targetOutboundPeers(
+		cfg.MaxPeers, permanentPeerCount, automaticOutbound,
+	)
+	if targetOutbound == 0 {
+		newAddressFunc = nil
+	}
 	reservedOutbound := reservedOutboundPeers(
 		cfg.MaxPeers, targetOutbound, permanentPeerCount,
-		newAddressFunc != nil,
+		automaticOutbound,
 	)
 	maxInbound := maxInboundPeers(cfg.MaxPeers, reservedOutbound)
 	if maxInbound == 0 && len(listeners) > 0 {

server_test.go

@@ -117,61 +117,66 @@ func TestInboundPeerReservation(t *testing.T) {
 	tests := []struct {
 		name           string
 		maxPeers       int
-		targetOutbound int
 		permanentPeers int
 		automatic      bool
+		wantTarget     int
 		wantReserved   int
 		wantInbound    uint32
 	}{
 		{
-			name: "connect only", maxPeers: 8, targetOutbound: 8,
+			name: "connect only", maxPeers: 8,
 			permanentPeers: 1, wantReserved: 1, wantInbound: 7,
 		},
 		{
 			name: "connect only capped", maxPeers: 8,
-			targetOutbound: 8, permanentPeers: 10,
+			permanentPeers: 10,
-			wantReserved: 8, wantInbound: 0,
+			wantReserved:   8, wantInbound: 0,
 		},
 		{
-			name: "simnet without peers", maxPeers: 8,
+			name: "simnet without peers", maxPeers: 8, wantReserved: 0,
-			targetOutbound: 8, wantReserved: 0, wantInbound: 8,
+			wantInbound: 8,
 		},
 		{
-			name: "simnet with peers", maxPeers: 8,
+			name: "simnet with peers", maxPeers: 8, permanentPeers: 3,
-			targetOutbound: 8, permanentPeers: 3,
 			wantReserved: 3, wantInbound: 5,
 		},
 		{
 			name: "automatic without add peers", maxPeers: 125,
-			targetOutbound: 8, automatic: true,
+			automatic: true, wantTarget: 8,
 			wantReserved: 8, wantInbound: 117,
 		},
 		{
 			name: "add peers below target", maxPeers: 125,
-			targetOutbound: 8, permanentPeers: 3, automatic: true,
+			permanentPeers: 3, automatic: true, wantTarget: 8,
 			wantReserved: 11, wantInbound: 114,
 		},
 		{
 			name: "add peers above target", maxPeers: 10,
-			targetOutbound: 8, permanentPeers: 9, automatic: true,
+			permanentPeers: 9, automatic: true, wantTarget: 1,
 			wantReserved: 10, wantInbound: 0,
 		},
 		{
 			name: "add peers at max peers", maxPeers: 10,
-			targetOutbound: 8, permanentPeers: 10, automatic: true,
+			permanentPeers: 10, automatic: true,
 			wantReserved: 10, wantInbound: 0,
 		},
 		{
 			name: "add peers above max peers", maxPeers: 10,
-			targetOutbound: 8, permanentPeers: 12, automatic: true,
+			permanentPeers: 12, automatic: true,
 			wantReserved: 10, wantInbound: 0,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
+			targetOutbound := targetOutboundPeers(
+				test.maxPeers, test.permanentPeers,
+				test.automatic,
+			)
+			require.Equal(t, test.wantTarget, targetOutbound)
+
 			reserved := reservedOutboundPeers(
-				test.maxPeers, test.targetOutbound,
+				test.maxPeers, targetOutbound,
 				test.permanentPeers, test.automatic,
 			)
 			require.Equal(t, test.wantReserved, reserved)