diff --git a/network/phonebook.go b/network/phonebook.go index ad07a2b5f5..0ad7be1a0c 100644 --- a/network/phonebook.go +++ b/network/phonebook.go @@ -68,8 +68,9 @@ type Phonebook interface { // matching entries don't change ReplacePeerList(dnsAddresses []string, networkName string, role PhoneBookEntryRoles) - // ExtendPeerList adds unique addresses to this set of addresses - ExtendPeerList(more []string, networkName string, role PhoneBookEntryRoles) + // AddPersistentPeers stores addresses of peers which are persistent. + // i.e. they won't be replaced by ReplacePeerList calls + AddPersistentPeers(dnsAddresses []string, networkName string, role PhoneBookEntryRoles) } // addressData: holds the information associated with each phonebook address. @@ -86,14 +87,18 @@ type addressData struct { // role is the role that this address serves. role PhoneBookEntryRoles + + // persistent is set true for peers whose record should not be removed for the peer list + persistent bool } // makePhonebookEntryData creates a new addressData entry for provided network name and role. -func makePhonebookEntryData(networkName string, role PhoneBookEntryRoles) addressData { +func makePhonebookEntryData(networkName string, role PhoneBookEntryRoles, persistent bool) addressData { pbData := addressData{ networkNames: make(map[string]bool), recentConnectionTimes: make([]time.Time, 0), role: role, + persistent: persistent, } pbData.networkNames[networkName] = true return pbData @@ -165,7 +170,7 @@ func (e *phonebookImpl) ReplacePeerList(addressesThey []string, networkName stri // prepare a map of items we'd like to remove. removeItems := make(map[string]bool, 0) for k, pbd := range e.data { - if pbd.networkNames[networkName] && pbd.role == role { + if pbd.networkNames[networkName] && pbd.role == role && !pbd.persistent { removeItems[k] = true } } @@ -180,7 +185,7 @@ func (e *phonebookImpl) ReplacePeerList(addressesThey []string, networkName stri delete(removeItems, addr) } else { // we don't have this item. add it. - e.data[addr] = makePhonebookEntryData(networkName, role) + e.data[addr] = makePhonebookEntryData(networkName, role, false) } } @@ -190,6 +195,23 @@ func (e *phonebookImpl) ReplacePeerList(addressesThey []string, networkName stri } } +func (e *phonebookImpl) AddPersistentPeers(dnsAddresses []string, networkName string, role PhoneBookEntryRoles) { + e.lock.Lock() + defer e.lock.Unlock() + + for _, addr := range dnsAddresses { + if pbData, has := e.data[addr]; has { + // we already have this. + // Make sure the persistence field is set to true + pbData.persistent = true + + } else { + // we don't have this item. add it. + e.data[addr] = makePhonebookEntryData(networkName, role, true) + } + } +} + func (e *phonebookImpl) UpdateRetryAfter(addr string, retryAfter time.Time) { e.lock.Lock() defer e.lock.Unlock() @@ -316,19 +338,6 @@ func (e *phonebookImpl) GetAddresses(n int, role PhoneBookEntryRoles) []string { return shuffleSelect(e.filterRetryTime(time.Now(), role), n) } -// ExtendPeerList adds unique addresses to this set of addresses -func (e *phonebookImpl) ExtendPeerList(more []string, networkName string, role PhoneBookEntryRoles) { - e.lock.Lock() - defer e.lock.Unlock() - for _, addr := range more { - if pbEntry, has := e.data[addr]; has { - pbEntry.networkNames[networkName] = true - continue - } - e.data[addr] = makePhonebookEntryData(networkName, role) - } -} - // Length returns the number of addrs contained func (e *phonebookImpl) Length() int { e.lock.RLock() diff --git a/network/phonebook_test.go b/network/phonebook_test.go index 64f0d7c03a..4fdad53c42 100644 --- a/network/phonebook_test.go +++ b/network/phonebook_test.go @@ -17,14 +17,10 @@ package network import ( - "fmt" - "math/rand" - "sync" "testing" "time" "github.com/algorand/go-algorand/test/partitiontest" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -92,7 +88,7 @@ func TestArrayPhonebookAll(t *testing.T) { set := []string{"a", "b", "c", "d", "e"} ph := MakePhonebook(1, 1).(*phonebookImpl) for _, e := range set { - ph.data[e] = makePhonebookEntryData("", PhoneBookEntryRelayRole) + ph.data[e] = makePhonebookEntryData("", PhoneBookEntryRelayRole, false) } testPhonebookAll(t, set, ph) } @@ -103,7 +99,7 @@ func TestArrayPhonebookUniform1(t *testing.T) { set := []string{"a", "b", "c", "d", "e"} ph := MakePhonebook(1, 1).(*phonebookImpl) for _, e := range set { - ph.data[e] = makePhonebookEntryData("", PhoneBookEntryRelayRole) + ph.data[e] = makePhonebookEntryData("", PhoneBookEntryRelayRole, false) } testPhonebookUniform(t, set, ph, 1) } @@ -114,91 +110,39 @@ func TestArrayPhonebookUniform3(t *testing.T) { set := []string{"a", "b", "c", "d", "e"} ph := MakePhonebook(1, 1).(*phonebookImpl) for _, e := range set { - ph.data[e] = makePhonebookEntryData("", PhoneBookEntryRelayRole) + ph.data[e] = makePhonebookEntryData("", PhoneBookEntryRelayRole, false) } testPhonebookUniform(t, set, ph, 3) } -// TestPhonebookExtension tests for extending different phonebooks with -// addresses. -func TestPhonebookExtension(t *testing.T) { - partitiontest.PartitionTest(t) - - setA := []string{"a"} - moreB := []string{"b"} - ph := MakePhonebook(1, 1).(*phonebookImpl) - ph.ReplacePeerList(setA, "default", PhoneBookEntryRelayRole) - ph.ExtendPeerList(moreB, "default", PhoneBookEntryRelayRole) - ph.ExtendPeerList(setA, "other", PhoneBookEntryRelayRole) - assert.Equal(t, 2, ph.Length()) - assert.Equal(t, true, ph.data["a"].networkNames["default"]) - assert.Equal(t, true, ph.data["a"].networkNames["other"]) - assert.Equal(t, true, ph.data["b"].networkNames["default"]) - assert.Equal(t, false, ph.data["b"].networkNames["other"]) -} - -func extenderThread(th *phonebookImpl, more []string, wg *sync.WaitGroup, repetitions int) { - defer wg.Done() - for i := 0; i <= repetitions; i++ { - start := rand.Intn(len(more)) - end := rand.Intn(len(more)-start) + start - th.ExtendPeerList(more[start:end], "default", PhoneBookEntryRelayRole) - } - th.ExtendPeerList(more, "default", PhoneBookEntryRelayRole) -} - -func TestThreadsafePhonebookExtension(t *testing.T) { - partitiontest.PartitionTest(t) - - set := []string{"a", "b", "c", "d", "e"} - more := []string{"f", "g", "h", "i", "j"} - ph := MakePhonebook(1, 1).(*phonebookImpl) - ph.ReplacePeerList(set, "default", PhoneBookEntryRelayRole) - wg := sync.WaitGroup{} - wg.Add(5) - for ti := 0; ti < 5; ti++ { - go extenderThread(ph, more, &wg, 1000) - } - wg.Wait() - - assert.Equal(t, 10, ph.Length()) -} - -func threadTestThreadsafePhonebookExtensionLong(wg *sync.WaitGroup, ph *phonebookImpl, setSize, repetitions int) { - set := make([]string, setSize) - for i := range set { - set[i] = fmt.Sprintf("%06d", i) - } - rand.Shuffle(len(set), func(i, j int) { t := set[i]; set[i] = set[j]; set[j] = t }) - extenderThread(ph, set, wg, repetitions) -} - -func TestThreadsafePhonebookExtensionLong(t *testing.T) { +func TestMultiPhonebook(t *testing.T) { partitiontest.PartitionTest(t) - if testing.Short() { - t.SkipNow() - return + set := []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"} + pha := make([]string, 0) + for _, e := range set[:5] { + pha = append(pha, e) } - ph := MakePhonebook(1, 1).(*phonebookImpl) - wg := sync.WaitGroup{} - const threads = 5 - const setSize = 1000 - const repetitions = 100 - wg.Add(threads) - for i := 0; i < threads; i++ { - go threadTestThreadsafePhonebookExtensionLong(&wg, ph, setSize, repetitions) + phb := make([]string, 0) + for _, e := range set[5:] { + phb = append(phb, e) } + mp := MakePhonebook(1, 1*time.Millisecond) + mp.ReplacePeerList(pha, "pha", PhoneBookEntryRelayRole) + mp.ReplacePeerList(phb, "phb", PhoneBookEntryRelayRole) - wg.Wait() - - assert.Equal(t, setSize, ph.Length()) + testPhonebookAll(t, set, mp) + testPhonebookUniform(t, set, mp, 1) + testPhonebookUniform(t, set, mp, 3) } -func TestMultiPhonebook(t *testing.T) { +// TestMultiPhonebookPersistentPeers validates that the peers added via Phonebook.AddPersistentPeers +// are not replaced when Phonebook.ReplacePeerList is called +func TestMultiPhonebookPersistentPeers(t *testing.T) { partitiontest.PartitionTest(t) - set := []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"} + persistentPeers := []string{"a"} + set := []string{"b", "c", "d", "e", "f", "g", "h", "i", "j", "k"} pha := make([]string, 0) for _, e := range set[:5] { pha = append(pha, e) @@ -208,12 +152,16 @@ func TestMultiPhonebook(t *testing.T) { phb = append(phb, e) } mp := MakePhonebook(1, 1*time.Millisecond) + mp.AddPersistentPeers(persistentPeers, "pha", PhoneBookEntryRelayRole) + mp.AddPersistentPeers(persistentPeers, "phb", PhoneBookEntryRelayRole) mp.ReplacePeerList(pha, "pha", PhoneBookEntryRelayRole) mp.ReplacePeerList(phb, "phb", PhoneBookEntryRelayRole) - testPhonebookAll(t, set, mp) - testPhonebookUniform(t, set, mp, 1) - testPhonebookUniform(t, set, mp, 3) + testPhonebookAll(t, append(set, persistentPeers...), mp) + allAddresses := mp.GetAddresses(len(set)+len(persistentPeers), PhoneBookEntryRelayRole) + for _, pp := range persistentPeers { + require.Contains(t, allAddresses, pp) + } } func TestMultiPhonebookDuplicateFiltering(t *testing.T) { @@ -388,21 +336,6 @@ func TestWaitAndAddConnectionTimeShortWindow(t *testing.T) { require.Equal(t, 1, len(phBookData)) } -func BenchmarkThreadsafePhonebook(b *testing.B) { - ph := MakePhonebook(1, 1).(*phonebookImpl) - threads := 5 - if b.N < threads { - threads = b.N - } - wg := sync.WaitGroup{} - wg.Add(threads) - repetitions := b.N / threads - for t := 0; t < threads; t++ { - go threadTestThreadsafePhonebookExtensionLong(&wg, ph, 1000, repetitions) - } - wg.Wait() -} - // TestPhonebookRoles tests that the filtering by roles for different // phonebooks entries works as expected. func TestPhonebookRoles(t *testing.T) { diff --git a/network/wsNetwork.go b/network/wsNetwork.go index 56314f14b9..028cee6341 100644 --- a/network/wsNetwork.go +++ b/network/wsNetwork.go @@ -2449,7 +2449,7 @@ func (wn *WebsocketNetwork) SetPeerData(peer Peer, key string, value interface{} func NewWebsocketNetwork(log logging.Logger, config config.Local, phonebookAddresses []string, genesisID string, networkID protocol.NetworkID, nodeInfo NodeInfo) (wn *WebsocketNetwork, err error) { phonebook := MakePhonebook(config.ConnectionsRateLimitingCount, time.Duration(config.ConnectionsRateLimitingWindowSeconds)*time.Second) - phonebook.ReplacePeerList(phonebookAddresses, string(networkID), PhoneBookEntryRelayRole) + phonebook.AddPersistentPeers(phonebookAddresses, string(networkID), PhoneBookEntryRelayRole) wn = &WebsocketNetwork{ log: log, config: config, diff --git a/network/wsNetwork_test.go b/network/wsNetwork_test.go index 05992ec8d6..487f46ec7a 100644 --- a/network/wsNetwork_test.go +++ b/network/wsNetwork_test.go @@ -4306,6 +4306,40 @@ func TestUpdatePhonebookAddresses(t *testing.T) { }) } +func TestUpdatePhonebookAddressesPersistentPeers(t *testing.T) { + partitiontest.PartitionTest(t) + + rapid.Check(t, func(t1 *rapid.T) { + nw := makeTestWebsocketNode(t) + // Generate a new set of relay domains + // Dont overlap with archive nodes previously specified, duplicates between them not stored in phonebook as of this writing + relayDomainsGen := rapid.SliceOfN(rapidgen.DomainOf(253, 63, "", nil), 0, 200) + relayDomains := relayDomainsGen.Draw(t1, "relayDomains") + + var persistentPeers []string + // Add an initial set of relay domains as Persistent Peers in the Phonebook, + persistentPeers = rapid.SliceOfN(rapidgen.DomainOf(253, 63, "", relayDomains), 0, 200).Draw(t1, "") + nw.phonebook.AddPersistentPeers(persistentPeers, string(nw.NetworkID), PhoneBookEntryRelayRole) + + // run updatePhonebookAddresses + nw.updatePhonebookAddresses(relayDomains, nil) + + // Check that entries are in fact in phonebook less any duplicates + dedupedRelayDomains := removeDuplicateStr(relayDomains, false) + require.Equal(t, 0, len(relayDomains)-len(dedupedRelayDomains)) + + relayPeers := nw.GetPeers(PeersPhonebookRelays) + require.Equal(t, len(dedupedRelayDomains)+len(persistentPeers), len(relayPeers)) + + relayAddrs := make([]string, 0, len(relayPeers)) + for _, peer := range relayPeers { + relayAddrs = append(relayAddrs, peer.(HTTPPeer).GetAddress()) + } + + require.ElementsMatch(t, append(dedupedRelayDomains, persistentPeers...), relayAddrs) + }) +} + func removeDuplicateStr(strSlice []string, lowerCase bool) []string { allKeys := make(map[string]bool) var dedupStrSlice = make([]string, 0)