[NOD-133] BTCD node connects multiple times to the same address (#263)

* [NOD-133] Added addrTrying.

* [NOD-133] Fixed infinite look inside getAddress.

* [NOD-133] Reverted log level to trace.

* [NOD-133] Fixed failing test.

* [NOD-133] Added an explanation as to why devnet is exempt from same-CIDR checking.

* [NOD-133] Changed config.DevNet with activeNetParams.AcceptUnroutable for same-CIDR checking.

addrmgr/addrmanager.go

@@ -44,6 +44,7 @@ type AddrManager struct {
 	addrNewFullNodes   newBucket
 	addrTried          map[subnetworkid.SubnetworkID]*triedBucket
 	addrTriedFullNodes triedBucket
+	addrTrying         map[*KnownAddress]bool
 	started            int32
 	shutdown           int32
 	wg                 sync.WaitGroup
@@ -889,6 +890,8 @@ func (a *AddrManager) reset() {
 	}
 	a.nNewFullNodes = 0
 	a.nTriedFullNodes = 0
+
+	a.addrTrying = make(map[*KnownAddress]bool)
 }
 
 // HostToNetAddress returns a netaddress given a host address.  If the address
@@ -952,15 +955,25 @@ func (a *AddrManager) GetAddress() *KnownAddress {
 	a.mtx.Lock()
 	defer a.mtx.Unlock()
 
+	var knownAddress *KnownAddress
 	if a.localSubnetworkID == nil {
-		return a.getAddress(&a.addrTriedFullNodes, a.nTriedFullNodes,
+		knownAddress = a.getAddress(&a.addrTriedFullNodes, a.nTriedFullNodes,
 			&a.addrNewFullNodes, a.nNewFullNodes)
+	} else {
+		subnetworkID := *a.localSubnetworkID
+		knownAddress = a.getAddress(a.addrTried[subnetworkID], a.nTried[subnetworkID],
+			a.addrNew[subnetworkID], a.nNew[subnetworkID])
 	}
 
-	subnetworkID := *a.localSubnetworkID
+	if knownAddress != nil {
+		if a.addrTrying[knownAddress] {
+			return nil
+		}
+		a.addrTrying[knownAddress] = true
+	}
+
+	return knownAddress
 
-	return a.getAddress(a.addrTried[subnetworkID], a.nTried[subnetworkID],
-		a.addrNew[subnetworkID], a.nNew[subnetworkID])
 }
 
 // see GetAddress for details
@@ -1043,6 +1056,8 @@ func (a *AddrManager) Attempt(addr *wire.NetAddress) {
 	// set last tried time to now
 	ka.attempts++
 	ka.lastattempt = time.Now()
+
+	delete(a.addrTrying, ka)
 }
 
 // Connected Marks the given address as currently connected and working at the

addrmgr/addrmanager_test.go

@@ -417,6 +417,7 @@ func TestGetAddress(t *testing.T) {
 	if ka == nil {
 		t.Fatalf("Did not get an address where there is one in the pool")
 	}
+	n.Attempt(ka.NetAddress())
 
 	// Checks that we don't get it if we find that it has other subnetwork ID than expected.
 	actualSubnetworkID := &subnetworkid.SubnetworkID{0xfe}
@@ -449,6 +450,7 @@ func TestGetAddress(t *testing.T) {
 	if !ka.SubnetworkID().IsEqual(localSubnetworkID) {
 		t.Errorf("Wrong Subnetwork ID: got %v, want %v", *ka.SubnetworkID(), localSubnetworkID)
 	}
+	n.Attempt(ka.NetAddress())
 
 	// Mark this as a good address and get it
 	n.Good(ka.NetAddress(), localSubnetworkID)

server/p2p/p2p.go

@@ -2523,9 +2523,15 @@ func NewServer(listenAddrs []string, db database.DB, dagParams *dagconfig.Params
 				// in the same group so that we are not connecting
 				// to the same network segment at the expense of
 				// others.
-				key := addrmgr.GroupKey(addr.NetAddress())
-				if s.OutboundGroupCount(key) != 0 {
-					continue
+				//
+				// Networks that accept unroutable connections are exempt
+				// from this rule, since they're meant to run within a
+				// private subnet, like 10.0.0.0/16.
+				if !config.ActiveNetParams().AcceptUnroutable {
+					key := addrmgr.GroupKey(addr.NetAddress())
+					if s.OutboundGroupCount(key) != 0 {
+						continue
+					}
 				}
 
 				// only allow recent nodes (10mins) after we failed 30