Enable gossiping of newer IPs when connected to an older IP (#3035)

network/README.md

@@ -173,7 +173,6 @@ A `GetPeerList` message contains the Bloom Filter of the currently known peers a
 `PeerList` messages are expected to contain `IP:Port` pairs that satisfy all of the following constraints:
 - The Bloom Filter sent when requesting the `PeerList` message does not contain the node claiming the `IP:Port` pair.
 - The node claiming the `IP:Port` pair is currently connected.
-- The `IP:Port` pair the node shared during the `Handshake` message is the node's most recently known `IP:Port` pair.
 - The node claiming the `IP:Port` pair is either in the default bootstrapper set, is a current Primary Network validator, is a validator of a tracked Subnet, or is a validator of a Subnet and the peer is a Primary Network validator.
 
 #### Example PeerList Gossip

network/ip_tracker.go

@@ -140,7 +140,12 @@ type gossipableSubnet struct {
 	gossipableIPs     []*ips.ClaimedIPPort
 }
 
-func (s *gossipableSubnet) addGossipableIP(ip *ips.ClaimedIPPort) {
+func (s *gossipableSubnet) setGossipableIP(ip *ips.ClaimedIPPort) {
+	if index, ok := s.gossipableIndices[ip.NodeID]; ok {
+		s.gossipableIPs[index] = ip
+		return
+	}
+
 	s.numGossipableIPs.Inc()
 	s.gossipableIndices[ip.NodeID] = len(s.gossipableIPs)
 	s.gossipableIPs = append(s.gossipableIPs, ip)
@@ -307,14 +312,21 @@ func (i *ipTracker) ShouldVerifyIP(
 //     subnet.
 //  2. This IP is newer than the most recent IP we know of for the node.
 //
-// If the previous IP for this node was marked as gossipable, calling this
-// function will remove the previous IP from the gossipable set.
+// If this IP is replacing a gossipable IP, this IP will also be marked as
+// gossipable.
 func (i *ipTracker) AddIP(ip *ips.ClaimedIPPort) bool {
 	i.lock.Lock()
 	defer i.lock.Unlock()
 
 	timestampComparison, trackedNode := i.addIP(ip)
-	return timestampComparison > sameTimestamp && trackedNode.wantsConnection()
+	if timestampComparison <= sameTimestamp {
+		return false
+	}
+
+	if connectedNode, ok := i.connected[ip.NodeID]; ok {
+		i.setGossipableIP(trackedNode.ip, connectedNode.trackedSubnets)
+	}
+	return trackedNode.wantsConnection()
 }
 
 // GetIP returns the most recent IP of the provided nodeID. Returns true if all
@@ -343,14 +355,10 @@ func (i *ipTracker) Connected(ip *ips.ClaimedIPPort, trackedSubnets set.Set[ids.
 		trackedSubnets: trackedSubnets,
 		ip:             ip,
 	}
-	if timestampComparison, _ := i.addIP(ip); timestampComparison < sameTimestamp {
-		return
-	}
 
-	for subnetID := range trackedSubnets {
-		if subnet, ok := i.subnet[subnetID]; ok && subnet.gossipableIDs.Contains(ip.NodeID) {
-			subnet.addGossipableIP(ip)
-		}
+	timestampComparison, trackedNode := i.addIP(ip)
+	if timestampComparison != untrackedTimestamp {
+		i.setGossipableIP(trackedNode.ip, trackedSubnets)
 	}
 }
 
@@ -364,8 +372,6 @@ func (i *ipTracker) addIP(ip *ips.ClaimedIPPort) (int, *trackedNode) {
 		// This is the first IP we've heard from the validator, so it is the
 		// most recent.
 		i.updateMostRecentTrackedIP(node, ip)
-		// Because we didn't previously have an IP, we know we aren't currently
-		// connected to them.
 		return newTimestamp, node
 	}
 
@@ -376,25 +382,29 @@ func (i *ipTracker) addIP(ip *ips.ClaimedIPPort) (int, *trackedNode) {
 		return sameTimestamp, node // This IP is equal to the previously known IP.
 	}
 
+	// This IP is newer than the previously known IP.
 	i.updateMostRecentTrackedIP(node, ip)
-	i.removeGossipableIP(ip.NodeID)
 	return newerTimestamp, node
 }
 
+func (i *ipTracker) setGossipableIP(ip *ips.ClaimedIPPort, trackedSubnets set.Set[ids.ID]) {
+	for subnetID := range trackedSubnets {
+		if subnet, ok := i.subnet[subnetID]; ok && subnet.gossipableIDs.Contains(ip.NodeID) {
+			subnet.setGossipableIP(ip)
+		}
+	}
+}
+
 // Disconnected is called when a connection to the peer is closed.
 func (i *ipTracker) Disconnected(nodeID ids.NodeID) {
 	i.lock.Lock()
 	defer i.lock.Unlock()
 
-	i.removeGossipableIP(nodeID)
-	delete(i.connected, nodeID)
-}
-
-func (i *ipTracker) removeGossipableIP(nodeID ids.NodeID) {
 	connectedNode, ok := i.connected[nodeID]
 	if !ok {
 		return
 	}
+	delete(i.connected, nodeID)
 
 	for subnetID := range connectedNode.trackedSubnets {
 		if subnet, ok := i.subnet[subnetID]; ok {
@@ -467,11 +477,9 @@ func (i *ipTracker) addGossipableID(nodeID ids.NodeID, subnetID ids.ID, manually
 		return
 	}
 
-	if trackedNode, ok := i.tracked[nodeID]; !ok || trackedNode.ip == nil || trackedNode.ip.Timestamp != node.ip.Timestamp {
-		return
+	if trackedNode, ok := i.tracked[nodeID]; ok {
+		subnet.setGossipableIP(trackedNode.ip)
 	}
-
-	subnet.addGossipableIP(node.ip)
 }
 
 func (*ipTracker) OnValidatorWeightChanged(ids.ID, ids.NodeID, uint64, uint64) {}

network/ip_tracker_test.go

@@ -544,13 +544,9 @@ func TestIPTracker_AddIP(t *testing.T) {
 			ip:                        newerIP,
 			expectedUpdatedAndDesired: true,
 			expectedChange: func(tracker *ipTracker) {
-				tracker.numGossipableIPs.Dec()
 				tracker.tracked[newerIP.NodeID].ip = newerIP
 				tracker.bloomAdditions[newerIP.NodeID] = 2
-
-				subnet := tracker.subnet[constants.PrimaryNetworkID]
-				delete(subnet.gossipableIndices, newerIP.NodeID)
-				subnet.gossipableIPs = subnet.gossipableIPs[:0]
+				tracker.subnet[constants.PrimaryNetworkID].gossipableIPs[0] = newerIP
 			},
 		},
 		{
@@ -655,10 +651,17 @@ func TestIPTracker_Connected(t *testing.T) {
 			},
 			ip: ip,
 			expectedChange: func(tracker *ipTracker) {
+				tracker.numGossipableIPs.Inc()
 				tracker.connected[ip.NodeID] = &connectedNode{
 					trackedSubnets: set.Of(constants.PrimaryNetworkID),
 					ip:             ip,
 				}
+
+				subnet := tracker.subnet[constants.PrimaryNetworkID]
+				subnet.gossipableIndices[newerIP.NodeID] = 0
+				subnet.gossipableIPs = []*ips.ClaimedIPPort{
+					newerIP,
+				}
 			},
 		},
 		{
@@ -930,12 +933,18 @@ func TestIPTracker_OnValidatorAdded(t *testing.T) {
 			},
 			subnetID: constants.PrimaryNetworkID,
 			expectedChange: func(tracker *ipTracker) {
+				tracker.numGossipableIPs.Inc()
 				tracker.tracked[ip.NodeID].subnets.Add(constants.PrimaryNetworkID)
 				tracker.tracked[ip.NodeID].trackedSubnets.Add(constants.PrimaryNetworkID)
 				tracker.subnet[constants.PrimaryNetworkID] = &gossipableSubnet{
-					numGossipableIPs:  tracker.numGossipableIPs,
-					gossipableIDs:     set.Of(ip.NodeID),
-					gossipableIndices: make(map[ids.NodeID]int),
+					numGossipableIPs: tracker.numGossipableIPs,
+					gossipableIDs:    set.Of(ip.NodeID),
+					gossipableIndices: map[ids.NodeID]int{
+						ip.NodeID: 0,
+					},
+					gossipableIPs: []*ips.ClaimedIPPort{
+						newerIP,
+					},
 				}
 			},
 		},