From 4ac0fa5a582c49cd70412a9919760a803332bced Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Fri, 12 Apr 2024 17:59:44 -0400 Subject: [PATCH 1/2] Split ManuallyTrack into ManuallyTrack and ManuallyGossip --- network/ip_tracker.go | 260 +++++++++++++++++----------- network/ip_tracker_test.go | 338 ++++++++++++++++++++++++++++--------- network/network.go | 2 +- 3 files changed, 427 insertions(+), 173 deletions(-) diff --git a/network/ip_tracker.go b/network/ip_tracker.go index 13f7133630d6..fb6526ead6f4 100644 --- a/network/ip_tracker.go +++ b/network/ip_tracker.go @@ -30,6 +30,12 @@ const ( // By setting maxIPEntriesPerValidator > 1, we allow validators to update // their IP at least once per bloom filter reset. maxIPEntriesPerValidator = 2 + + untrackedTimestamp = -2 + olderTimestamp = -1 + sameTimestamp = 0 + newerTimestamp = 1 + newTimestamp = 2 ) var _ validators.SetCallbackListener = (*ipTracker)(nil) @@ -46,25 +52,25 @@ func newIPTracker( } tracker := &ipTracker{ log: log, - numValidatorIPs: prometheus.NewGauge(prometheus.GaugeOpts{ + numTrackedIPs: prometheus.NewGauge(prometheus.GaugeOpts{ Namespace: namespace, - Name: "validator_ips", - Help: "Number of known validator IPs", + Name: "tracked_ips", + Help: "Number of IPs this node is willing to dial", }), - numGossipable: prometheus.NewGauge(prometheus.GaugeOpts{ + numGossipableIPs: prometheus.NewGauge(prometheus.GaugeOpts{ Namespace: namespace, Name: "gossipable_ips", Help: "Number of IPs this node is willing to gossip", }), - bloomMetrics: bloomMetrics, - connected: make(map[ids.NodeID]*ips.ClaimedIPPort), - mostRecentValidatorIPs: make(map[ids.NodeID]*ips.ClaimedIPPort), - gossipableIndices: make(map[ids.NodeID]int), - bloomAdditions: make(map[ids.NodeID]int), + bloomMetrics: bloomMetrics, + mostRecentTrackedIPs: make(map[ids.NodeID]*ips.ClaimedIPPort), + bloomAdditions: make(map[ids.NodeID]int), + connected: make(map[ids.NodeID]*ips.ClaimedIPPort), + gossipableIndices: make(map[ids.NodeID]int), } err = utils.Err( - registerer.Register(tracker.numValidatorIPs), - registerer.Register(tracker.numGossipable), + registerer.Register(tracker.numTrackedIPs), + registerer.Register(tracker.numGossipableIPs), ) if err != nil { return nil, err @@ -73,29 +79,31 @@ func newIPTracker( } type ipTracker struct { - log logging.Logger - numValidatorIPs prometheus.Gauge - numGossipable prometheus.Gauge - bloomMetrics *bloom.Metrics + log logging.Logger + numTrackedIPs prometheus.Gauge + numGossipableIPs prometheus.Gauge + bloomMetrics *bloom.Metrics lock sync.RWMutex - // Manually tracked nodes are always treated like validators + // manuallyTracked contains the nodeIDs of all nodes whose connection was + // manually requested. manuallyTracked set.Set[ids.NodeID] - // Connected tracks the currently connected peers, including validators and - // non-validators. The IP is not necessarily the same IP as in - // mostRecentIPs. - connected map[ids.NodeID]*ips.ClaimedIPPort - mostRecentValidatorIPs map[ids.NodeID]*ips.ClaimedIPPort - validators set.Set[ids.NodeID] - - // An IP is marked as gossipable if: + // manuallyGossipable contains the nodeIDs of all nodes whose IP was + // manually configured to be gossiped. + manuallyGossipable set.Set[ids.NodeID] + + // mostRecentTrackedIPs tracks the most recent IP of each node whose + // connection is desired. + // + // An IP is tracked if one of the following conditions are met: + // - The node was manually tracked + // - The node was manually requested to be gossiped // - The node is a validator - // - The node is connected - // - The IP the node connected with is its latest IP - gossipableIndices map[ids.NodeID]int - gossipableIPs []*ips.ClaimedIPPort + mostRecentTrackedIPs map[ids.NodeID]*ips.ClaimedIPPort + // trackedIDs contains the nodeIDs of all nodes whose connection is desired. + trackedIDs set.Set[ids.NodeID] - // The bloom filter contains the most recent validator IPs to avoid + // The bloom filter contains the most recent tracked IPs to avoid // unnecessary IP gossip. bloom *bloom.Filter // To prevent validators from causing the bloom filter to have too many @@ -104,109 +112,146 @@ type ipTracker struct { bloomAdditions map[ids.NodeID]int // Number of IPs added to the bloom bloomSalt []byte maxBloomCount int + + // Connected tracks the IP of currently connected peers, including tracked + // and untracked nodes. The IP is not necessarily the same IP as in + // mostRecentTrackedIPs. + connected map[ids.NodeID]*ips.ClaimedIPPort + + // An IP is marked as gossipable if all of the following conditions are met: + // - The node is a validator or was manually requested to be gossiped + // - The node is connected + // - The IP the node connected with is its latest IP + gossipableIndices map[ids.NodeID]int + // gossipableIPs is guaranteed to be a subset of [mostRecentTrackedIPs]. + gossipableIPs []*ips.ClaimedIPPort + gossipableIDs set.Set[ids.NodeID] } +// ManuallyTrack marks the provided nodeID as being desirable to connect to. +// +// In order for a node to learn about these nodeIDs, other nodes in the network +// must have marked them as gossipable. +// +// Even if nodes disagree on the set of manually tracked nodeIDs, they will not +// introduce persistent network gossip. func (i *ipTracker) ManuallyTrack(nodeID ids.NodeID) { i.lock.Lock() defer i.lock.Unlock() - // We treat manually tracked nodes as if they were validators. - if !i.validators.Contains(nodeID) { - i.onValidatorAdded(nodeID) - } - // Now that the node is marked as a validator, freeze it's validation - // status. Future calls to OnValidatorAdded or OnValidatorRemoved will be - // treated as noops. + i.addTrackableID(nodeID) + i.manuallyTracked.Add(nodeID) +} + +// ManuallyGossip marks the provided nodeID as being desirable to connect to and +// marks the IPs that this node provides as being valid to gossip. +// +// In order to avoid persistent network gossip, it's important for nodes in the +// network to agree upon manually gossiped nodeIDs. +func (i *ipTracker) ManuallyGossip(nodeID ids.NodeID) { + i.lock.Lock() + defer i.lock.Unlock() + + i.addTrackableID(nodeID) i.manuallyTracked.Add(nodeID) + + i.addGossipableID(nodeID) + i.manuallyGossipable.Add(nodeID) } +// WantsConnection returns true if any of the following conditions are met: +// 1. The node has been manually tracked. +// 2. The node has been manually gossipped. +// 3. The node is currently a validator. func (i *ipTracker) WantsConnection(nodeID ids.NodeID) bool { i.lock.RLock() defer i.lock.RUnlock() - return i.validators.Contains(nodeID) + return i.trackedIDs.Contains(nodeID) } +// ShouldVerifyIP is used as an optimization to avoid unnecessary IP +// verification. It returns true if all of the following conditions are met: +// 1. The provided IP is from a node whose connection is desired. +// 2. This IP is newer than the most recent IP we know of for the node. func (i *ipTracker) ShouldVerifyIP(ip *ips.ClaimedIPPort) bool { i.lock.RLock() defer i.lock.RUnlock() - if !i.validators.Contains(ip.NodeID) { + if !i.trackedIDs.Contains(ip.NodeID) { return false } - prevIP, ok := i.mostRecentValidatorIPs[ip.NodeID] + prevIP, ok := i.mostRecentTrackedIPs[ip.NodeID] return !ok || // This would be the first IP prevIP.Timestamp < ip.Timestamp // This would be a newer IP } -// AddIP returns true if the addition of the provided IP updated the most -// recently known IP of a validator. +// AddIP attempts to update the node's IP to the provided IP. This function +// assumes the provided IP has been verified. Returns true if all of the +// following conditions are met: +// 1. The provided IP is from a node whose connection is desired. +// 2. This IP is newer than the most recent IP we know of for the node. +// +// If the previous IP was marked as gossipable, calling this function will +// remove the IP from the gossipable set. func (i *ipTracker) AddIP(ip *ips.ClaimedIPPort) bool { i.lock.Lock() defer i.lock.Unlock() - if !i.validators.Contains(ip.NodeID) { - return false - } - - prevIP, ok := i.mostRecentValidatorIPs[ip.NodeID] - if !ok { - // This is the first IP we've heard from the validator, so it is the - // most recent. - i.updateMostRecentValidatorIP(ip) - // Because we didn't previously have an IP, we know we aren't currently - // connected to them. - return true - } - - if prevIP.Timestamp >= ip.Timestamp { - // This IP is not newer than the previously known IP. - return false - } - - i.updateMostRecentValidatorIP(ip) - i.removeGossipableIP(ip.NodeID) - return true + return i.addIP(ip) > sameTimestamp } +// GetIP returns the most recent IP of the provided nodeID. If a connection to +// this nodeID is not desired, this function will return false. func (i *ipTracker) GetIP(nodeID ids.NodeID) (*ips.ClaimedIPPort, bool) { i.lock.RLock() defer i.lock.RUnlock() - ip, ok := i.mostRecentValidatorIPs[nodeID] + ip, ok := i.mostRecentTrackedIPs[nodeID] return ip, ok } +// Connected is called when a connection is established. The peer should have +// provided [ip] during the handshake. func (i *ipTracker) Connected(ip *ips.ClaimedIPPort) { i.lock.Lock() defer i.lock.Unlock() i.connected[ip.NodeID] = ip - if !i.validators.Contains(ip.NodeID) { - return + if i.addIP(ip) >= sameTimestamp && i.gossipableIDs.Contains(ip.NodeID) { + i.addGossipableIP(ip) + } +} + +func (i *ipTracker) addIP(ip *ips.ClaimedIPPort) int { + if !i.trackedIDs.Contains(ip.NodeID) { + return untrackedTimestamp } - prevIP, ok := i.mostRecentValidatorIPs[ip.NodeID] + prevIP, ok := i.mostRecentTrackedIPs[ip.NodeID] if !ok { // This is the first IP we've heard from the validator, so it is the // most recent. - i.updateMostRecentValidatorIP(ip) - i.addGossipableIP(ip) - return + i.updateMostRecentTrackedIP(ip) + // Because we didn't previously have an IP, we know we aren't currently + // connected to them. + return newTimestamp } if prevIP.Timestamp > ip.Timestamp { - // There is a more up-to-date IP than the one that was used to connect. - return + return olderTimestamp // This IP is old than the previously known IP. } - - if prevIP.Timestamp < ip.Timestamp { - i.updateMostRecentValidatorIP(ip) + if prevIP.Timestamp == ip.Timestamp { + return sameTimestamp // This IP is equal to the previously known IP. } - i.addGossipableIP(ip) + + i.updateMostRecentTrackedIP(ip) + i.removeGossipableIP(ip.NodeID) + return newerTimestamp } +// 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() @@ -219,24 +264,42 @@ func (i *ipTracker) OnValidatorAdded(nodeID ids.NodeID, _ *bls.PublicKey, _ ids. i.lock.Lock() defer i.lock.Unlock() - i.onValidatorAdded(nodeID) + i.addTrackableID(nodeID) + i.addGossipableID(nodeID) } -func (i *ipTracker) onValidatorAdded(nodeID ids.NodeID) { - if i.manuallyTracked.Contains(nodeID) { +func (i *ipTracker) addTrackableID(nodeID ids.NodeID) { + if i.trackedIDs.Contains(nodeID) { return } - i.validators.Add(nodeID) + i.trackedIDs.Add(nodeID) ip, connected := i.connected[nodeID] if !connected { return } - // Because we only track validator IPs, the from the connection is - // guaranteed to be the most up-to-date IP that we know. - i.updateMostRecentValidatorIP(ip) - i.addGossipableIP(ip) + // Because we previously weren't tracking this nodeID, the IP from the + // connection is guaranteed to be the most up-to-date IP that we know. + i.updateMostRecentTrackedIP(ip) +} + +func (i *ipTracker) addGossipableID(nodeID ids.NodeID) { + if i.gossipableIDs.Contains(nodeID) { + return + } + + i.gossipableIDs.Add(nodeID) + connectedIP, connected := i.connected[nodeID] + if !connected { + return + } + + if updatedIP, ok := i.mostRecentTrackedIPs[nodeID]; !ok || connectedIP.Timestamp != updatedIP.Timestamp { + return + } + + i.addGossipableIP(connectedIP) } func (*ipTracker) OnValidatorWeightChanged(ids.NodeID, uint64, uint64) {} @@ -245,20 +308,25 @@ func (i *ipTracker) OnValidatorRemoved(nodeID ids.NodeID, _ uint64) { i.lock.Lock() defer i.lock.Unlock() - if i.manuallyTracked.Contains(nodeID) { + if i.manuallyGossipable.Contains(nodeID) { return } - delete(i.mostRecentValidatorIPs, nodeID) - i.numValidatorIPs.Set(float64(len(i.mostRecentValidatorIPs))) - - i.validators.Remove(nodeID) + i.gossipableIDs.Remove(nodeID) i.removeGossipableIP(nodeID) + + if i.manuallyTracked.Contains(nodeID) { + return + } + + i.trackedIDs.Remove(nodeID) + delete(i.mostRecentTrackedIPs, nodeID) + i.numTrackedIPs.Set(float64(len(i.mostRecentTrackedIPs))) } -func (i *ipTracker) updateMostRecentValidatorIP(ip *ips.ClaimedIPPort) { - i.mostRecentValidatorIPs[ip.NodeID] = ip - i.numValidatorIPs.Set(float64(len(i.mostRecentValidatorIPs))) +func (i *ipTracker) updateMostRecentTrackedIP(ip *ips.ClaimedIPPort) { + i.mostRecentTrackedIPs[ip.NodeID] = ip + i.numTrackedIPs.Set(float64(len(i.mostRecentTrackedIPs))) oldCount := i.bloomAdditions[ip.NodeID] if oldCount >= maxIPEntriesPerValidator { @@ -290,7 +358,7 @@ func (i *ipTracker) updateMostRecentValidatorIP(ip *ips.ClaimedIPPort) { func (i *ipTracker) addGossipableIP(ip *ips.ClaimedIPPort) { i.gossipableIndices[ip.NodeID] = len(i.gossipableIPs) i.gossipableIPs = append(i.gossipableIPs, ip) - i.numGossipable.Inc() + i.numGossipableIPs.Inc() } func (i *ipTracker) removeGossipableIP(nodeID ids.NodeID) { @@ -309,7 +377,7 @@ func (i *ipTracker) removeGossipableIP(nodeID ids.NodeID) { delete(i.gossipableIndices, nodeID) i.gossipableIPs[newNumGossipable] = nil i.gossipableIPs = i.gossipableIPs[:newNumGossipable] - i.numGossipable.Dec() + i.numGossipableIPs.Dec() } // GetGossipableIPs returns the latest IPs of connected validators. The returned @@ -378,7 +446,7 @@ func (i *ipTracker) resetBloom() error { return err } - count := max(maxIPEntriesPerValidator*i.validators.Len(), minCountEstimate) + count := max(maxIPEntriesPerValidator*i.gossipableIDs.Len(), minCountEstimate) numHashes, numEntries := bloom.OptimalParameters( count, targetFalsePositiveProbability, @@ -393,7 +461,7 @@ func (i *ipTracker) resetBloom() error { i.bloomSalt = newSalt i.maxBloomCount = bloom.EstimateCount(numHashes, numEntries, maxFalsePositiveProbability) - for nodeID, ip := range i.mostRecentValidatorIPs { + for nodeID, ip := range i.mostRecentTrackedIPs { bloom.Add(newFilter, ip.GossipID[:], newSalt) i.bloomAdditions[nodeID] = 1 } diff --git a/network/ip_tracker_test.go b/network/ip_tracker_test.go index 0bfda448c72a..c3a27e2445ff 100644 --- a/network/ip_tracker_test.go +++ b/network/ip_tracker_test.go @@ -34,19 +34,21 @@ func newerTestIP(ip *ips.ClaimedIPPort) *ips.ClaimedIPPort { func requireEqual(t *testing.T, expected, actual *ipTracker) { require := require.New(t) require.Equal(expected.manuallyTracked, actual.manuallyTracked) + require.Equal(expected.manuallyGossipable, actual.manuallyGossipable) + require.Equal(expected.mostRecentTrackedIPs, actual.mostRecentTrackedIPs) + require.Equal(expected.trackedIDs, actual.trackedIDs) + require.Equal(expected.bloomAdditions, actual.bloomAdditions) + require.Equal(expected.maxBloomCount, actual.maxBloomCount) require.Equal(expected.connected, actual.connected) - require.Equal(expected.mostRecentValidatorIPs, actual.mostRecentValidatorIPs) - require.Equal(expected.validators, actual.validators) require.Equal(expected.gossipableIndices, actual.gossipableIndices) require.Equal(expected.gossipableIPs, actual.gossipableIPs) - require.Equal(expected.bloomAdditions, actual.bloomAdditions) - require.Equal(expected.maxBloomCount, actual.maxBloomCount) + require.Equal(expected.gossipableIDs, actual.gossipableIDs) } func requireMetricsConsistent(t *testing.T, tracker *ipTracker) { require := require.New(t) - require.Equal(float64(len(tracker.mostRecentValidatorIPs)), testutil.ToFloat64(tracker.numValidatorIPs)) - require.Equal(float64(len(tracker.gossipableIPs)), testutil.ToFloat64(tracker.numGossipable)) + require.Equal(float64(len(tracker.mostRecentTrackedIPs)), testutil.ToFloat64(tracker.numTrackedIPs)) + require.Equal(float64(len(tracker.gossipableIPs)), testutil.ToFloat64(tracker.numGossipableIPs)) require.Equal(float64(tracker.bloom.Count()), testutil.ToFloat64(tracker.bloomMetrics.Count)) require.Equal(float64(tracker.maxBloomCount), testutil.ToFloat64(tracker.bloomMetrics.MaxCount)) } @@ -64,8 +66,8 @@ func TestIPTracker_ManuallyTrack(t *testing.T) { nodeID: ip.NodeID, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.validators.Add(ip.NodeID) tracker.manuallyTracked.Add(ip.NodeID) + tracker.trackedIDs.Add(ip.NodeID) return tracker }(), }, @@ -80,14 +82,96 @@ func TestIPTracker_ManuallyTrack(t *testing.T) { expectedState: func() *ipTracker { tracker := newTestIPTracker(t) tracker.Connected(ip) - tracker.mostRecentValidatorIPs[ip.NodeID] = ip + tracker.manuallyTracked.Add(ip.NodeID) + tracker.mostRecentTrackedIPs[ip.NodeID] = ip + tracker.trackedIDs.Add(ip.NodeID) + tracker.bloomAdditions[ip.NodeID] = 1 + return tracker + }(), + }, + { + name: "non-connected validator", + initialState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + return tracker + }(), + nodeID: ip.NodeID, + expectedState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + tracker.manuallyTracked.Add(ip.NodeID) + return tracker + }(), + }, + { + name: "connected validator", + initialState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.Connected(ip) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + return tracker + }(), + nodeID: ip.NodeID, + expectedState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.Connected(ip) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + tracker.manuallyTracked.Add(ip.NodeID) + return tracker + }(), + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + test.initialState.ManuallyTrack(test.nodeID) + requireEqual(t, test.expectedState, test.initialState) + requireMetricsConsistent(t, test.initialState) + }) + } +} + +func TestIPTracker_ManuallyGossip(t *testing.T) { + tests := []struct { + name string + initialState *ipTracker + nodeID ids.NodeID + expectedState *ipTracker + }{ + { + name: "non-connected non-validator", + initialState: newTestIPTracker(t), + nodeID: ip.NodeID, + expectedState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.manuallyTracked.Add(ip.NodeID) + tracker.manuallyGossipable.Add(ip.NodeID) + tracker.trackedIDs.Add(ip.NodeID) + tracker.gossipableIDs.Add(ip.NodeID) + return tracker + }(), + }, + { + name: "connected non-validator", + initialState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.Connected(ip) + return tracker + }(), + nodeID: ip.NodeID, + expectedState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.Connected(ip) + tracker.manuallyTracked.Add(ip.NodeID) + tracker.manuallyGossipable.Add(ip.NodeID) + tracker.mostRecentTrackedIPs[ip.NodeID] = ip + tracker.trackedIDs.Add(ip.NodeID) tracker.bloomAdditions[ip.NodeID] = 1 tracker.gossipableIndices[ip.NodeID] = 0 tracker.gossipableIPs = []*ips.ClaimedIPPort{ ip, } - tracker.validators.Add(ip.NodeID) - tracker.manuallyTracked.Add(ip.NodeID) + tracker.gossipableIDs.Add(ip.NodeID) return tracker }(), }, @@ -95,14 +179,15 @@ func TestIPTracker_ManuallyTrack(t *testing.T) { name: "non-connected validator", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) return tracker }(), nodeID: ip.NodeID, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.manuallyTracked.Add(ip.NodeID) + tracker.manuallyGossipable.Add(ip.NodeID) return tracker }(), }, @@ -111,22 +196,23 @@ func TestIPTracker_ManuallyTrack(t *testing.T) { initialState: func() *ipTracker { tracker := newTestIPTracker(t) tracker.Connected(ip) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) return tracker }(), nodeID: ip.NodeID, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) tracker.Connected(ip) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.manuallyTracked.Add(ip.NodeID) + tracker.manuallyGossipable.Add(ip.NodeID) return tracker }(), }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - test.initialState.ManuallyTrack(test.nodeID) + test.initialState.ManuallyGossip(test.nodeID) requireEqual(t, test.expectedState, test.initialState) requireMetricsConsistent(t, test.initialState) }) @@ -153,15 +239,15 @@ func TestIPTracker_AddIP(t *testing.T) { name: "first known IP", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) return tracker }(), ip: ip, expectedUpdated: true, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) - tracker.mostRecentValidatorIPs[ip.NodeID] = ip + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + tracker.mostRecentTrackedIPs[ip.NodeID] = ip tracker.bloomAdditions[ip.NodeID] = 1 return tracker }(), @@ -170,7 +256,7 @@ func TestIPTracker_AddIP(t *testing.T) { name: "older IP", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(newerIP.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(newerIP)) return tracker }(), @@ -178,7 +264,7 @@ func TestIPTracker_AddIP(t *testing.T) { expectedUpdated: false, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(newerIP.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(newerIP)) return tracker }(), @@ -187,7 +273,7 @@ func TestIPTracker_AddIP(t *testing.T) { name: "same IP", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(ip)) return tracker }(), @@ -195,7 +281,7 @@ func TestIPTracker_AddIP(t *testing.T) { expectedUpdated: false, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(ip)) return tracker }(), @@ -204,7 +290,7 @@ func TestIPTracker_AddIP(t *testing.T) { name: "disconnected newer IP", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(ip)) return tracker }(), @@ -212,9 +298,9 @@ func TestIPTracker_AddIP(t *testing.T) { expectedUpdated: true, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(ip)) - tracker.mostRecentValidatorIPs[newerIP.NodeID] = newerIP + tracker.mostRecentTrackedIPs[newerIP.NodeID] = newerIP tracker.bloomAdditions[newerIP.NodeID] = 2 return tracker }(), @@ -223,7 +309,7 @@ func TestIPTracker_AddIP(t *testing.T) { name: "connected newer IP", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) return tracker }(), @@ -231,9 +317,9 @@ func TestIPTracker_AddIP(t *testing.T) { expectedUpdated: true, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) - tracker.mostRecentValidatorIPs[newerIP.NodeID] = newerIP + tracker.mostRecentTrackedIPs[newerIP.NodeID] = newerIP tracker.bloomAdditions[newerIP.NodeID] = 2 delete(tracker.gossipableIndices, newerIP.NodeID) tracker.gossipableIPs = tracker.gossipableIPs[:0] @@ -273,16 +359,16 @@ func TestIPTracker_Connected(t *testing.T) { name: "first known IP", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) return tracker }(), ip: ip, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) - tracker.connected[ip.NodeID] = ip - tracker.mostRecentValidatorIPs[ip.NodeID] = ip + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + tracker.mostRecentTrackedIPs[ip.NodeID] = ip tracker.bloomAdditions[ip.NodeID] = 1 + tracker.connected[ip.NodeID] = ip tracker.gossipableIndices[ip.NodeID] = 0 tracker.gossipableIPs = []*ips.ClaimedIPPort{ ip, @@ -294,14 +380,14 @@ func TestIPTracker_Connected(t *testing.T) { name: "connected with older IP", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(newerIP.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(newerIP)) return tracker }(), ip: ip, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(newerIP.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(newerIP)) tracker.connected[ip.NodeID] = ip return tracker @@ -311,18 +397,18 @@ func TestIPTracker_Connected(t *testing.T) { name: "connected with newer IP", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(ip)) return tracker }(), ip: newerIP, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(ip)) - tracker.connected[newerIP.NodeID] = newerIP - tracker.mostRecentValidatorIPs[newerIP.NodeID] = newerIP + tracker.mostRecentTrackedIPs[newerIP.NodeID] = newerIP tracker.bloomAdditions[newerIP.NodeID] = 2 + tracker.connected[newerIP.NodeID] = newerIP tracker.gossipableIndices[newerIP.NodeID] = 0 tracker.gossipableIPs = []*ips.ClaimedIPPort{ newerIP, @@ -334,14 +420,14 @@ func TestIPTracker_Connected(t *testing.T) { name: "connected with same IP", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(ip)) return tracker }(), ip: ip, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(ip)) tracker.connected[ip.NodeID] = ip tracker.gossipableIndices[ip.NodeID] = 0 @@ -369,7 +455,7 @@ func TestIPTracker_Disconnected(t *testing.T) { expectedState *ipTracker }{ { - name: "not gossipable", + name: "not tracked", initialState: func() *ipTracker { tracker := newTestIPTracker(t) tracker.Connected(ip) @@ -378,18 +464,35 @@ func TestIPTracker_Disconnected(t *testing.T) { nodeID: ip.NodeID, expectedState: newTestIPTracker(t), }, + { + name: "not gossipable", + initialState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.Connected(ip) + tracker.ManuallyTrack(ip.NodeID) + return tracker + }(), + nodeID: ip.NodeID, + expectedState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.Connected(ip) + tracker.ManuallyTrack(ip.NodeID) + delete(tracker.connected, ip.NodeID) + return tracker + }(), + }, { name: "latest gossipable", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) return tracker }(), nodeID: ip.NodeID, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) delete(tracker.connected, ip.NodeID) delete(tracker.gossipableIndices, ip.NodeID) @@ -401,18 +504,18 @@ func TestIPTracker_Disconnected(t *testing.T) { name: "non-latest gossipable", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) - tracker.onValidatorAdded(otherIP.NodeID) + tracker.OnValidatorAdded(otherIP.NodeID, nil, ids.Empty, 0) tracker.Connected(otherIP) return tracker }(), nodeID: ip.NodeID, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) - tracker.onValidatorAdded(otherIP.NodeID) + tracker.OnValidatorAdded(otherIP.NodeID, nil, ids.Empty, 0) tracker.Connected(otherIP) delete(tracker.connected, ip.NodeID) tracker.gossipableIndices = map[ids.NodeID]int{ @@ -435,6 +538,8 @@ func TestIPTracker_Disconnected(t *testing.T) { } func TestIPTracker_OnValidatorAdded(t *testing.T) { + newerIP := newerTestIP(ip) + tests := []struct { name string initialState *ipTracker @@ -452,6 +557,40 @@ func TestIPTracker_OnValidatorAdded(t *testing.T) { expectedState: func() *ipTracker { tracker := newTestIPTracker(t) tracker.ManuallyTrack(ip.NodeID) + tracker.gossipableIDs.Add(ip.NodeID) + return tracker + }(), + }, + { + name: "manually tracked and connected with older IP", + initialState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.ManuallyTrack(ip.NodeID) + tracker.Connected(ip) + require.True(t, tracker.AddIP(newerIP)) + return tracker + }(), + nodeID: ip.NodeID, + expectedState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.ManuallyTrack(ip.NodeID) + tracker.Connected(ip) + require.True(t, tracker.AddIP(newerIP)) + tracker.gossipableIDs.Add(ip.NodeID) + return tracker + }(), + }, + { + name: "manually gossipped", + initialState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.ManuallyGossip(ip.NodeID) + return tracker + }(), + nodeID: ip.NodeID, + expectedState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.ManuallyGossip(ip.NodeID) return tracker }(), }, @@ -461,7 +600,8 @@ func TestIPTracker_OnValidatorAdded(t *testing.T) { nodeID: ip.NodeID, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.validators.Add(ip.NodeID) + tracker.trackedIDs.Add(ip.NodeID) + tracker.gossipableIDs.Add(ip.NodeID) return tracker }(), }, @@ -476,13 +616,14 @@ func TestIPTracker_OnValidatorAdded(t *testing.T) { expectedState: func() *ipTracker { tracker := newTestIPTracker(t) tracker.Connected(ip) - tracker.validators.Add(ip.NodeID) - tracker.mostRecentValidatorIPs[ip.NodeID] = ip + tracker.mostRecentTrackedIPs[ip.NodeID] = ip + tracker.trackedIDs.Add(ip.NodeID) tracker.bloomAdditions[ip.NodeID] = 1 tracker.gossipableIndices[ip.NodeID] = 0 tracker.gossipableIPs = []*ips.ClaimedIPPort{ ip, } + tracker.gossipableIDs.Add(ip.NodeID) return tracker }(), }, @@ -504,11 +645,30 @@ func TestIPTracker_OnValidatorRemoved(t *testing.T) { expectedState *ipTracker }{ { - name: "manually tracked", + name: "manually tracked not gossipable", + initialState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.ManuallyTrack(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + require.True(t, tracker.AddIP(ip)) + return tracker + }(), + nodeID: ip.NodeID, + expectedState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.ManuallyTrack(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + require.True(t, tracker.AddIP(ip)) + tracker.gossipableIDs.Remove(ip.NodeID) + return tracker + }(), + }, + { + name: "manually tracked latest gossipable", initialState: func() *ipTracker { tracker := newTestIPTracker(t) tracker.ManuallyTrack(ip.NodeID) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) return tracker }(), @@ -516,7 +676,28 @@ func TestIPTracker_OnValidatorRemoved(t *testing.T) { expectedState: func() *ipTracker { tracker := newTestIPTracker(t) tracker.ManuallyTrack(ip.NodeID) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + tracker.Connected(ip) + delete(tracker.gossipableIndices, ip.NodeID) + tracker.gossipableIPs = tracker.gossipableIPs[:0] + tracker.gossipableIDs.Remove(ip.NodeID) + return tracker + }(), + }, + { + name: "manually gossipped", + initialState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.ManuallyGossip(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + tracker.Connected(ip) + return tracker + }(), + nodeID: ip.NodeID, + expectedState: func() *ipTracker { + tracker := newTestIPTracker(t) + tracker.ManuallyGossip(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) return tracker }(), @@ -525,17 +706,18 @@ func TestIPTracker_OnValidatorRemoved(t *testing.T) { name: "not gossipable", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(ip)) return tracker }(), nodeID: ip.NodeID, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) require.True(t, tracker.AddIP(ip)) - delete(tracker.mostRecentValidatorIPs, ip.NodeID) - tracker.validators.Remove(ip.NodeID) + delete(tracker.mostRecentTrackedIPs, ip.NodeID) + tracker.trackedIDs.Remove(ip.NodeID) + tracker.gossipableIDs.Remove(ip.NodeID) return tracker }(), }, @@ -543,19 +725,20 @@ func TestIPTracker_OnValidatorRemoved(t *testing.T) { name: "latest gossipable", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) return tracker }(), nodeID: ip.NodeID, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) - delete(tracker.mostRecentValidatorIPs, ip.NodeID) - tracker.validators.Remove(ip.NodeID) + delete(tracker.mostRecentTrackedIPs, ip.NodeID) + tracker.trackedIDs.Remove(ip.NodeID) delete(tracker.gossipableIndices, ip.NodeID) tracker.gossipableIPs = tracker.gossipableIPs[:0] + tracker.gossipableIDs.Remove(ip.NodeID) return tracker }(), }, @@ -563,27 +746,28 @@ func TestIPTracker_OnValidatorRemoved(t *testing.T) { name: "non-latest gossipable", initialState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) - tracker.onValidatorAdded(otherIP.NodeID) + tracker.OnValidatorAdded(otherIP.NodeID, nil, ids.Empty, 0) tracker.Connected(otherIP) return tracker }(), nodeID: ip.NodeID, expectedState: func() *ipTracker { tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.Connected(ip) - tracker.onValidatorAdded(otherIP.NodeID) + tracker.OnValidatorAdded(otherIP.NodeID, nil, ids.Empty, 0) tracker.Connected(otherIP) - delete(tracker.mostRecentValidatorIPs, ip.NodeID) - tracker.validators.Remove(ip.NodeID) + delete(tracker.mostRecentTrackedIPs, ip.NodeID) + tracker.trackedIDs.Remove(ip.NodeID) tracker.gossipableIndices = map[ids.NodeID]int{ otherIP.NodeID: 0, } tracker.gossipableIPs = []*ips.ClaimedIPPort{ otherIP, } + tracker.gossipableIDs.Remove(ip.NodeID) return tracker }(), }, @@ -603,8 +787,8 @@ func TestIPTracker_GetGossipableIPs(t *testing.T) { tracker := newTestIPTracker(t) tracker.Connected(ip) tracker.Connected(otherIP) - tracker.onValidatorAdded(ip.NodeID) - tracker.onValidatorAdded(otherIP.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + tracker.OnValidatorAdded(otherIP.NodeID, nil, ids.Empty, 0) gossipableIPs := tracker.GetGossipableIPs(ids.EmptyNodeID, bloom.EmptyFilter, nil, 2) require.ElementsMatch([]*ips.ClaimedIPPort{ip, otherIP}, gossipableIPs) @@ -632,8 +816,8 @@ func TestIPTracker_BloomFiltersEverything(t *testing.T) { tracker := newTestIPTracker(t) tracker.Connected(ip) tracker.Connected(otherIP) - tracker.onValidatorAdded(ip.NodeID) - tracker.onValidatorAdded(otherIP.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) + tracker.OnValidatorAdded(otherIP.NodeID, nil, ids.Empty, 0) bloomBytes, salt := tracker.Bloom() readFilter, err := bloom.Parse(bloomBytes) @@ -651,7 +835,7 @@ func TestIPTracker_BloomGrowsWithValidatorSet(t *testing.T) { tracker := newTestIPTracker(t) initialMaxBloomCount := tracker.maxBloomCount for i := 0; i < 2048; i++ { - tracker.onValidatorAdded(ids.GenerateTestNodeID()) + tracker.OnValidatorAdded(ids.GenerateTestNodeID(), nil, ids.Empty, 0) } requireMetricsConsistent(t, tracker) @@ -665,11 +849,11 @@ func TestIPTracker_BloomResetsDynamically(t *testing.T) { tracker := newTestIPTracker(t) tracker.Connected(ip) - tracker.onValidatorAdded(ip.NodeID) + tracker.OnValidatorAdded(ip.NodeID, nil, ids.Empty, 0) tracker.OnValidatorRemoved(ip.NodeID, 0) tracker.maxBloomCount = 1 tracker.Connected(otherIP) - tracker.onValidatorAdded(otherIP.NodeID) + tracker.OnValidatorAdded(otherIP.NodeID, nil, ids.Empty, 0) requireMetricsConsistent(t, tracker) bloomBytes, salt := tracker.Bloom() @@ -687,7 +871,7 @@ func TestIPTracker_PreventBloomFilterAddition(t *testing.T) { newestIP := newerTestIP(newerIP) tracker := newTestIPTracker(t) - tracker.onValidatorAdded(ip.NodeID) + tracker.ManuallyGossip(ip.NodeID) require.True(tracker.AddIP(ip)) require.True(tracker.AddIP(newerIP)) require.True(tracker.AddIP(newestIP)) @@ -702,7 +886,9 @@ func TestIPTracker_ShouldVerifyIP(t *testing.T) { tracker := newTestIPTracker(t) require.False(tracker.ShouldVerifyIP(ip)) - tracker.onValidatorAdded(ip.NodeID) + tracker.ManuallyTrack(ip.NodeID) + require.True(tracker.ShouldVerifyIP(ip)) + tracker.ManuallyGossip(ip.NodeID) require.True(tracker.ShouldVerifyIP(ip)) require.True(tracker.AddIP(ip)) require.False(tracker.ShouldVerifyIP(ip)) diff --git a/network/network.go b/network/network.go index 5d34458272d2..1ff6057069e1 100644 --- a/network/network.go +++ b/network/network.go @@ -244,7 +244,7 @@ func NewNetwork( // Track all default bootstrappers to ensure their current IPs are gossiped // like validator IPs. for _, bootstrapper := range genesis.GetBootstrappers(config.NetworkID) { - ipTracker.ManuallyTrack(bootstrapper.ID) + ipTracker.ManuallyGossip(bootstrapper.ID) } peerConfig := &peer.Config{ From e48c6d369fbe913f79bf356db4484cb1e9f1129f Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Fri, 12 Apr 2024 18:04:02 -0400 Subject: [PATCH 2/2] typo --- network/ip_tracker.go | 2 +- network/ip_tracker_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/network/ip_tracker.go b/network/ip_tracker.go index fb6526ead6f4..0737c993954c 100644 --- a/network/ip_tracker.go +++ b/network/ip_tracker.go @@ -161,7 +161,7 @@ func (i *ipTracker) ManuallyGossip(nodeID ids.NodeID) { // WantsConnection returns true if any of the following conditions are met: // 1. The node has been manually tracked. -// 2. The node has been manually gossipped. +// 2. The node has been manually gossiped. // 3. The node is currently a validator. func (i *ipTracker) WantsConnection(nodeID ids.NodeID) bool { i.lock.RLock() diff --git a/network/ip_tracker_test.go b/network/ip_tracker_test.go index c3a27e2445ff..e60213ef86fc 100644 --- a/network/ip_tracker_test.go +++ b/network/ip_tracker_test.go @@ -581,7 +581,7 @@ func TestIPTracker_OnValidatorAdded(t *testing.T) { }(), }, { - name: "manually gossipped", + name: "manually gossiped", initialState: func() *ipTracker { tracker := newTestIPTracker(t) tracker.ManuallyGossip(ip.NodeID) @@ -685,7 +685,7 @@ func TestIPTracker_OnValidatorRemoved(t *testing.T) { }(), }, { - name: "manually gossipped", + name: "manually gossiped", initialState: func() *ipTracker { tracker := newTestIPTracker(t) tracker.ManuallyGossip(ip.NodeID)