Network: Archival node DNS Resolution (#5940)HEADmaster

7 files changed, 51 insertions, 55 deletions

diff --git a/catchup/peerSelector.go b/catchup/peerSelector.go
index 148529558..05556bb24 100644
--- a/catchup/peerSelector.go
+++ b/catchup/peerSelector.go
@@ -29,12 +29,12 @@ import (
 )
 
 const (
-	// peerRankInitialFirstPriority is the high-priority peers group ( typically, archivers )
+	// peerRankInitialFirstPriority is the high-priority peers group
 	peerRankInitialFirstPriority = 0
 	peerRank0LowBlockTime        = 1
 	peerRank0HighBlockTime       = 199
 
-	// peerRankInitialSecondPriority is the second priority peers group ( typically, relays )
+	// peerRankInitialSecondPriority is the second priority peers group
 	peerRankInitialSecondPriority = 200
 	peerRank1LowBlockTime         = 201
 	peerRank1HighBlockTime        = 399
diff --git a/config/localTemplate.go b/config/localTemplate.go
index 618facfd2..ce4294de0 100644
--- a/config/localTemplate.go
+++ b/config/localTemplate.go
@@ -171,6 +171,7 @@ type Local struct {
 	RestWriteTimeoutSeconds int `version[4]:"120"`
 
 	// DNSBootstrapID specifies the names of a set of DNS SRV records that identify the set of nodes available to connect to.
+	// This is applicable to both relay and archival nodes - they are assumed to use the same DNSBootstrapID today.
 	// When resolving the bootstrap ID <network> will be replaced by the genesis block's network name. This string uses a URL
 	// parsing library and supports optional backup and dedup parameters. 'backup' is used to provide a second DNS entry to use
 	// in case the primary is unavailable. dedup is intended to be used to deduplicate SRV records returned from the primary
diff --git a/network/gossipNode.go b/network/gossipNode.go
index 7a916fda3..3ac5cc7df 100644
--- a/network/gossipNode.go
+++ b/network/gossipNode.go
@@ -42,8 +42,6 @@ const (
 	PeersPhonebookRelays PeerOption = iota
 	// PeersPhonebookArchivalNodes specifies all archival nodes (relay or p2p)
 	PeersPhonebookArchivalNodes PeerOption = iota
-	// PeersPhonebookArchivers specifies all archivers in the phonebook
-	PeersPhonebookArchivers PeerOption = iota
 )
 
 // GossipNode represents a node in the gossip network
diff --git a/network/phonebook.go b/network/phonebook.go
index 3f196e060..0c431fd2f 100644
--- a/network/phonebook.go
+++ b/network/phonebook.go
@@ -30,7 +30,7 @@ import (
 const getAllAddresses = math.MaxInt32
 
 // PhoneBookEntryRoles defines the roles that a single entry on the phonebook can take.
-// currently, we have two roles : relay role and archiver role, which are mutually exclusive.
+// currently, we have two roles : relay role and archival role, which are mutually exclusive.
 //
 //msgp:ignore PhoneBookEntryRoles
 type PhoneBookEntryRoles int
@@ -39,8 +39,8 @@ type PhoneBookEntryRoles int
 // or via a configuration file.
 const PhoneBookEntryRelayRole = 1
 
-// PhoneBookEntryArchiverRole used for all the archivers that are provided via the archive SRV record.
-const PhoneBookEntryArchiverRole = 2
+// PhoneBookEntryArchivalRole used for all the archival nodes that are provided via the archive SRV record.
+const PhoneBookEntryArchivalRole = 2
 
 // Phonebook stores or looks up addresses of nodes we might contact
 type Phonebook interface {
diff --git a/network/phonebook_test.go b/network/phonebook_test.go
index 36365c591..2643e722e 100644
--- a/network/phonebook_test.go
+++ b/network/phonebook_test.go
@@ -346,11 +346,11 @@ func TestPhonebookRoles(t *testing.T) {
 
 	ph := MakePhonebook(1, 1).(*phonebookImpl)
 	ph.ReplacePeerList(relaysSet, "default", PhoneBookEntryRelayRole)
-	ph.ReplacePeerList(archiverSet, "default", PhoneBookEntryArchiverRole)
+	ph.ReplacePeerList(archiverSet, "default", PhoneBookEntryArchivalRole)
 	require.Equal(t, len(relaysSet)+len(archiverSet), len(ph.data))
 	require.Equal(t, len(relaysSet)+len(archiverSet), ph.Length())
 
-	for _, role := range []PhoneBookEntryRoles{PhoneBookEntryRelayRole, PhoneBookEntryArchiverRole} {
+	for _, role := range []PhoneBookEntryRoles{PhoneBookEntryRelayRole, PhoneBookEntryArchivalRole} {
 		for k := 0; k < 100; k++ {
 			for l := 0; l < 3; l++ {
 				entries := ph.GetAddresses(l, role)
@@ -358,7 +358,7 @@ func TestPhonebookRoles(t *testing.T) {
 					for _, entry := range entries {
 						require.Contains(t, entry, "relay")
 					}
-				} else if role == PhoneBookEntryArchiverRole {
+				} else if role == PhoneBookEntryArchivalRole {
 					for _, entry := range entries {
 						require.Contains(t, entry, "archiver")
 					}
diff --git a/network/wsNetwork.go b/network/wsNetwork.go
index 92a02976b..295bd7115 100644
--- a/network/wsNetwork.go
+++ b/network/wsNetwork.go
@@ -548,15 +548,7 @@ func (wn *WebsocketNetwork) GetPeers(options ...PeerOption) []Peer {
 			}
 		case PeersPhonebookArchivalNodes:
 			var addrs []string
-			addrs = wn.phonebook.GetAddresses(1000, PhoneBookEntryRelayRole)
-			for _, addr := range addrs {
-				peerCore := makePeerCore(wn.ctx, wn, wn.log, wn.handler.readBuffer, addr, wn.GetRoundTripper(), "" /*origin address*/)
-				outPeers = append(outPeers, &peerCore)
-			}
-		case PeersPhonebookArchivers:
-			// return copy of phonebook, which probably also contains peers we're connected to, but if it doesn't maybe we shouldn't be making new connections to those peers (because they disappeared from the directory)
-			var addrs []string
-			addrs = wn.phonebook.GetAddresses(1000, PhoneBookEntryArchiverRole)
+			addrs = wn.phonebook.GetAddresses(1000, PhoneBookEntryArchivalRole)
 			for _, addr := range addrs {
 				peerCore := makePeerCore(wn.ctx, wn, wn.log, wn.handler.readBuffer, addr, wn.GetRoundTripper(), "" /*origin address*/)
 				outPeers = append(outPeers, &peerCore)
@@ -1607,15 +1599,17 @@ func (wn *WebsocketNetwork) refreshRelayArchivePhonebookAddresses() {
 	dnsBootstrapArray := wn.config.DNSBootstrapArray(wn.NetworkID)
 
 	for _, dnsBootstrap := range dnsBootstrapArray {
-		primaryRelayAddrs, primaryArchiveAddrs := wn.getDNSAddrs(dnsBootstrap.PrimarySRVBootstrap)
+		primaryRelayAddrs, primaryArchivalAddrs := wn.getDNSAddrs(dnsBootstrap.PrimarySRVBootstrap)
 
 		if dnsBootstrap.BackupSRVBootstrap != "" {
-			backupRelayAddrs, backupArchiveAddrs := wn.getDNSAddrs(dnsBootstrap.BackupSRVBootstrap)
-			dedupedRelayAddresses := wn.mergePrimarySecondaryRelayAddressSlices(wn.NetworkID, primaryRelayAddrs,
+			backupRelayAddrs, backupArchivalAddrs := wn.getDNSAddrs(dnsBootstrap.BackupSRVBootstrap)
+			dedupedRelayAddresses := wn.mergePrimarySecondaryAddressSlices(primaryRelayAddrs,
 				backupRelayAddrs, dnsBootstrap.DedupExp)
-			wn.updatePhonebookAddresses(dedupedRelayAddresses, append(primaryArchiveAddrs, backupArchiveAddrs...))
+			dedupedArchivalAddresses := wn.mergePrimarySecondaryAddressSlices(primaryArchivalAddrs,
+				backupArchivalAddrs, dnsBootstrap.DedupExp)
+			wn.updatePhonebookAddresses(dedupedRelayAddresses, dedupedArchivalAddresses)
 		} else {
-			wn.updatePhonebookAddresses(primaryRelayAddrs, primaryArchiveAddrs)
+			wn.updatePhonebookAddresses(primaryRelayAddrs, primaryArchivalAddrs)
 		}
 	}
 }
@@ -1628,7 +1622,9 @@ func (wn *WebsocketNetwork) updatePhonebookAddresses(relayAddrs []string, archiv
 		wn.log.Infof("got no relay DNS addrs for network %s", wn.NetworkID)
 	}
 	if len(archiveAddrs) > 0 {
-		wn.phonebook.ReplacePeerList(archiveAddrs, string(wn.NetworkID), PhoneBookEntryArchiverRole)
+		wn.phonebook.ReplacePeerList(archiveAddrs, string(wn.NetworkID), PhoneBookEntryArchivalRole)
+	} else {
+		wn.log.Infof("got no archive DNS addrs for network %s", wn.NetworkID)
 	}
 }
 
@@ -1846,46 +1842,46 @@ func (wn *WebsocketNetwork) prioWeightRefresh() {
 	}
 }
 
-// This logic assumes that the relay address suffixes
+// This logic assumes that the address suffixes
 // correspond to the primary/backup network conventions. If this proves to be false, i.e. one network's
 // suffix is a substring of another network's suffix, then duplicates can end up in the merged slice.
-func (wn *WebsocketNetwork) mergePrimarySecondaryRelayAddressSlices(network protocol.NetworkID,
-	primaryRelayAddresses []string, secondaryRelayAddresses []string, dedupExp *regexp.Regexp) (dedupedRelayAddresses []string) {
+func (wn *WebsocketNetwork) mergePrimarySecondaryAddressSlices(
+	primaryAddresses []string, secondaryAddresses []string, dedupExp *regexp.Regexp) (dedupedAddresses []string) {
 
 	if dedupExp == nil {
 		// No expression provided, so just append the slices without deduping
-		return append(primaryRelayAddresses, secondaryRelayAddresses...)
+		return append(primaryAddresses, secondaryAddresses...)
 	}
 
-	var relayAddressPrefixToValue = make(map[string]string, 2*len(primaryRelayAddresses))
+	var addressPrefixToValue = make(map[string]string, 2*len(primaryAddresses))
 
-	for _, pra := range primaryRelayAddresses {
+	for _, pra := range primaryAddresses {
 		var normalizedPra = strings.ToLower(pra)
 
 		var pfxKey = dedupExp.ReplaceAllString(normalizedPra, "")
-		if _, exists := relayAddressPrefixToValue[pfxKey]; !exists {
-			relayAddressPrefixToValue[pfxKey] = normalizedPra
+		if _, exists := addressPrefixToValue[pfxKey]; !exists {
+			addressPrefixToValue[pfxKey] = normalizedPra
 		}
 	}
 
-	for _, sra := range secondaryRelayAddresses {
+	for _, sra := range secondaryAddresses {
 		var normalizedSra = strings.ToLower(sra)
 		var pfxKey = dedupExp.ReplaceAllString(normalizedSra, "")
 
-		if _, exists := relayAddressPrefixToValue[pfxKey]; !exists {
-			relayAddressPrefixToValue[pfxKey] = normalizedSra
+		if _, exists := addressPrefixToValue[pfxKey]; !exists {
+			addressPrefixToValue[pfxKey] = normalizedSra
 		}
 	}
 
-	dedupedRelayAddresses = make([]string, 0, len(relayAddressPrefixToValue))
-	for _, value := range relayAddressPrefixToValue {
-		dedupedRelayAddresses = append(dedupedRelayAddresses, value)
+	dedupedAddresses = make([]string, 0, len(addressPrefixToValue))
+	for _, value := range addressPrefixToValue {
+		dedupedAddresses = append(dedupedAddresses, value)
 	}
 
 	return
 }
 
-func (wn *WebsocketNetwork) getDNSAddrs(dnsBootstrap string) (relaysAddresses []string, archiverAddresses []string) {
+func (wn *WebsocketNetwork) getDNSAddrs(dnsBootstrap string) (relaysAddresses []string, archivalAddresses []string) {
 	var err error
 	relaysAddresses, err = wn.resolveSRVRecords(wn.ctx, "algobootstrap", "tcp", dnsBootstrap, wn.config.FallbackDNSResolverAddress, wn.config.DNSSecuritySRVEnforced())
 	if err != nil {
@@ -1896,13 +1892,13 @@ func (wn *WebsocketNetwork) getDNSAddrs(dnsBootstrap string) (relaysAddresses []
 		relaysAddresses = nil
 	}
 
-	archiverAddresses, err = wn.resolveSRVRecords(wn.ctx, "archive", "tcp", dnsBootstrap, wn.config.FallbackDNSResolverAddress, wn.config.DNSSecuritySRVEnforced())
+	archivalAddresses, err = wn.resolveSRVRecords(wn.ctx, "archive", "tcp", dnsBootstrap, wn.config.FallbackDNSResolverAddress, wn.config.DNSSecuritySRVEnforced())
 	if err != nil {
 		// only log this warning on testnet or devnet
 		if wn.NetworkID == config.Devnet || wn.NetworkID == config.Testnet {
 			wn.log.Warnf("Cannot lookup archive SRV record for %s: %v", dnsBootstrap, err)
 		}
-		archiverAddresses = nil
+		archivalAddresses = nil
 	}
 	return
 }
diff --git a/network/wsNetwork_test.go b/network/wsNetwork_test.go
index ef5769bcf..8daf4d196 100644
--- a/network/wsNetwork_test.go
+++ b/network/wsNetwork_test.go
@@ -1183,6 +1183,9 @@ func TestGetPeers(t *testing.T) {
 
 	phbMulti.ReplacePeerList([]string{"a", "b", "c"}, "ph", PhoneBookEntryRelayRole)
 
+	// A few for archival node roles
+	phbMulti.ReplacePeerList([]string{"d", "e", "f"}, "ph", PhoneBookEntryArchivalRole)
+
 	//addrB, _ := netB.Address()
 
 	// A has only an inbound connection from B
@@ -1206,14 +1209,13 @@ func TestGetPeers(t *testing.T) {
 	sort.Strings(expectAddrs)
 	assert.Equal(t, expectAddrs, peerAddrs)
 
-	// For now, PeersPhonebookArchivalNodes and PeersPhonebookRelays will return the same set of nodes
 	bPeers2 := netB.GetPeers(PeersPhonebookArchivalNodes)
 	peerAddrs2 := make([]string, len(bPeers2))
 	for pi2, peer2 := range bPeers2 {
 		peerAddrs2[pi2] = peer2.(HTTPPeer).GetAddress()
 	}
 	sort.Strings(peerAddrs2)
-	assert.Equal(t, expectAddrs, peerAddrs2)
+	assert.Equal(t, []string{"d", "e", "f"}, peerAddrs2)
 
 }
 
@@ -4176,7 +4178,7 @@ func TestRefreshRelayArchivePhonebookAddresses(t *testing.T) {
 		relayPeers := netA.GetPeers(PeersPhonebookRelays)
 		assert.Equal(t, 0, len(relayPeers))
 
-		archivePeers := netA.GetPeers(PeersPhonebookArchivers)
+		archivePeers := netA.GetPeers(PeersPhonebookArchivalNodes)
 		assert.Equal(t, 0, len(archivePeers))
 
 		netA.refreshRelayArchivePhonebookAddresses()
@@ -4191,17 +4193,16 @@ func TestRefreshRelayArchivePhonebookAddresses(t *testing.T) {
 
 		assert.ElementsMatch(t, primaryRelayResolvedRecords, relayAddrs)
 
-		archivePeers = netA.GetPeers(PeersPhonebookArchivers)
+		archivePeers = netA.GetPeers(PeersPhonebookArchivalNodes)
 
-		// TODO: For the time being, we do not dedup resolved archive nodes
-		assert.Equal(t, len(primaryArchiveResolvedRecords)+len(secondaryArchiveResolvedRecords), len(archivePeers))
+		assert.Equal(t, 3, len(archivePeers))
 
 		archiveAddrs := make([]string, 0, len(archivePeers))
 		for _, peer := range archivePeers {
 			archiveAddrs = append(archiveAddrs, peer.(HTTPPeer).GetAddress())
 		}
 
-		assert.ElementsMatch(t, append(primaryArchiveResolvedRecords, secondaryArchiveResolvedRecords...), archiveAddrs)
+		assert.ElementsMatch(t, primaryArchiveResolvedRecords, archiveAddrs)
 
 	})
 }
@@ -4219,7 +4220,7 @@ func TestUpdatePhonebookAddresses(t *testing.T) {
 		relayPeers := netA.GetPeers(PeersPhonebookRelays)
 		assert.Equal(t, 0, len(relayPeers))
 
-		archivePeers := netA.GetPeers(PeersPhonebookArchivers)
+		archivePeers := netA.GetPeers(PeersPhonebookArchivalNodes)
 		assert.Equal(t, 0, len(archivePeers))
 
 		domainGen := rapidgen.Domain()
@@ -4248,7 +4249,7 @@ func TestUpdatePhonebookAddresses(t *testing.T) {
 
 		assert.ElementsMatch(t, dedupedRelayDomains, relayAddrs)
 
-		archivePeers = netA.GetPeers(PeersPhonebookArchivers)
+		archivePeers = netA.GetPeers(PeersPhonebookArchivalNodes)
 		assert.Equal(t, len(dedupedArchiveDomains), len(archivePeers))
 
 		archiveAddrs := make([]string, 0, len(archivePeers))
@@ -4288,7 +4289,7 @@ func TestUpdatePhonebookAddresses(t *testing.T) {
 
 		assert.ElementsMatch(t, dedupedRelayDomains, relayAddrs)
 
-		archivePeers = netA.GetPeers(PeersPhonebookArchivers)
+		archivePeers = netA.GetPeers(PeersPhonebookArchivalNodes)
 		assert.Equal(t, len(dedupedArchiveDomains), len(archivePeers))
 
 		archiveAddrs = nil
@@ -4349,7 +4350,7 @@ func TestMergePrimarySecondaryRelayAddressListsMinOverlap(t *testing.T) {
 		primaryRelayAddresses := domainsGen.Draw(t1, "primaryRelayAddresses")
 		secondaryRelayAddresses := domainsGen.Draw(t1, "secondaryRelayAddresses")
 
-		mergedRelayAddresses := netA.mergePrimarySecondaryRelayAddressSlices(protocol.NetworkID(network),
+		mergedRelayAddresses := netA.mergePrimarySecondaryAddressSlices(
 			primaryRelayAddresses, secondaryRelayAddresses, dedupExp)
 
 		expectedRelayAddresses := removeDuplicateStr(append(primaryRelayAddresses, secondaryRelayAddresses...), true)
@@ -4402,7 +4403,7 @@ func TestMergePrimarySecondaryRelayAddressListsPartialOverlap(t *testing.T) {
 		}
 		secondaryRelayAddresses = append(secondaryRelayAddresses, extraSecondaryRelayAddresses...)
 
-		mergedRelayAddresses := netA.mergePrimarySecondaryRelayAddressSlices(network,
+		mergedRelayAddresses := netA.mergePrimarySecondaryAddressSlices(
 			primaryRelayAddresses, secondaryRelayAddresses, dedupExp)
 
 		// We expect the primary addresses to take precedence over a "matching" secondary address, extra non-duplicate
@@ -4445,7 +4446,7 @@ func TestMergePrimarySecondaryRelayAddressListsNoDedupExp(t *testing.T) {
 		generatedSecondaryRelayAddresses := secondaryDomainsGen.Draw(t1, "secondaryRelayAddresses")
 		secondaryRelayAddresses = append(secondaryRelayAddresses, generatedSecondaryRelayAddresses...)
 
-		mergedRelayAddresses := netA.mergePrimarySecondaryRelayAddressSlices(protocol.NetworkID(network),
+		mergedRelayAddresses := netA.mergePrimarySecondaryAddressSlices(
 			primaryRelayAddresses, secondaryRelayAddresses, nil)
 
 		// We expect non deduplication, so all addresses _should_ be present (note that no lower casing happens either)