Make public addresses go first in authority discovery DHT records (#3757)

Make sure explicitly set by the operator public addresses go first in
the authority discovery DHT records.

Also update `Discovery` behavior to eliminate duplicates in the returned
addresses.

This PR should improve situation with
https://github.com/paritytech/polkadot-sdk/issues/3519.

Obsoletes https://github.com/paritytech/polkadot-sdk/pull/3657.

substrate/client/authority-discovery/src/lib.rs

@@ -80,6 +80,10 @@ pub struct WorkerConfig {
 	/// Defaults to `true` to avoid the surprise factor.
 	pub publish_non_global_ips: bool,
 
+	/// Public addresses set by the node operator to always publish first in the authority
+	/// discovery DHT record.
+	pub public_addresses: Vec<Multiaddr>,
+
 	/// Reject authority discovery records that are not signed by their network identity (PeerId)
 	///
 	/// Defaults to `false` to provide compatibility with old versions
@@ -104,6 +108,7 @@ impl Default for WorkerConfig {
 			// `authority_discovery_dht_event_received`.
 			max_query_interval: Duration::from_secs(10 * 60),
 			publish_non_global_ips: true,
+			public_addresses: Vec::new(),
 			strict_record_validation: false,
 		}
 	}

substrate/client/authority-discovery/src/worker.rs

@@ -35,6 +35,7 @@ use addr_cache::AddrCache;
 use codec::{Decode, Encode};
 use ip_network::IpNetwork;
 use libp2p::{core::multiaddr, identity::PublicKey, multihash::Multihash, Multiaddr, PeerId};
+use linked_hash_set::LinkedHashSet;
 use multihash_codetable::{Code, MultihashDigest};
 
 use log::{debug, error, log_enabled};
@@ -120,14 +121,22 @@ pub struct Worker<Client, Network, Block, DhtEventStream> {
 
 	/// Interval to be proactive, publishing own addresses.
 	publish_interval: ExpIncInterval,
+
 	/// Pro-actively publish our own addresses at this interval, if the keys in the keystore
 	/// have changed.
 	publish_if_changed_interval: ExpIncInterval,
+
 	/// List of keys onto which addresses have been published at the latest publication.
 	/// Used to check whether they have changed.
 	latest_published_keys: HashSet<AuthorityId>,
+
 	/// Same value as in the configuration.
 	publish_non_global_ips: bool,
+
+	/// Public addresses set by the node operator to always publish first in the authority
+	/// discovery DHT record.
+	public_addresses: LinkedHashSet<Multiaddr>,
+
 	/// Same value as in the configuration.
 	strict_record_validation: bool,
 
@@ -136,6 +145,7 @@ pub struct Worker<Client, Network, Block, DhtEventStream> {
 
 	/// Queue of throttled lookups pending to be passed to the network.
 	pending_lookups: Vec<AuthorityId>,
+
 	/// Set of in-flight lookups.
 	in_flight_lookups: HashMap<KademliaKey, AuthorityId>,
 
@@ -224,6 +234,29 @@ where
 			None => None,
 		};
 
+		let public_addresses = {
+			let local_peer_id: Multihash = network.local_peer_id().into();
+
+			config
+				.public_addresses
+				.into_iter()
+				.map(|mut address| {
+					if let Some(multiaddr::Protocol::P2p(peer_id)) = address.iter().last() {
+						if peer_id != local_peer_id {
+							error!(
+								target: LOG_TARGET,
+								"Discarding invalid local peer ID in public address {address}.",
+							);
+						}
+						// Always discard `/p2p/...` protocol for proper address comparison (local
+						// peer id will be added before publishing).
+						address.pop();
+					}
+					address
+				})
+				.collect()
+		};
+
 		Worker {
 			from_service: from_service.fuse(),
 			client,
@@ -233,6 +266,7 @@ where
 			publish_if_changed_interval,
 			latest_published_keys: HashSet::new(),
 			publish_non_global_ips: config.publish_non_global_ips,
+			public_addresses,
 			strict_record_validation: config.strict_record_validation,
 			query_interval,
 			pending_lookups: Vec::new(),
@@ -304,32 +338,48 @@ where
 	}
 
 	fn addresses_to_publish(&self) -> impl Iterator<Item = Multiaddr> {
-		let peer_id: Multihash = self.network.local_peer_id().into();
 		let publish_non_global_ips = self.publish_non_global_ips;
-		let addresses = self.network.external_addresses().into_iter().filter(move |a| {
-			if publish_non_global_ips {
-				return true
-			}
+		let addresses = self
+			.public_addresses
+			.clone()
+			.into_iter()
+			.chain(self.network.external_addresses().into_iter().filter_map(|mut address| {
+				// Make sure the reported external address does not contain `/p2p/...` protocol.
+				if let Some(multiaddr::Protocol::P2p(_)) = address.iter().last() {
+					address.pop();
+				}
 
-			a.iter().all(|p| match p {
-				// The `ip_network` library is used because its `is_global()` method is stable,
-				// while `is_global()` in the standard library currently isn't.
-				multiaddr::Protocol::Ip4(ip) if !IpNetwork::from(ip).is_global() => false,
-				multiaddr::Protocol::Ip6(ip) if !IpNetwork::from(ip).is_global() => false,
-				_ => true,
+				if self.public_addresses.contains(&address) {
+					// Already added above.
+					None
+				} else {
+					Some(address)
+				}
+			}))
+			.filter(move |address| {
+				if publish_non_global_ips {
+					return true
+				}
+
+				address.iter().all(|protocol| match protocol {
+					// The `ip_network` library is used because its `is_global()` method is stable,
+					// while `is_global()` in the standard library currently isn't.
+					multiaddr::Protocol::Ip4(ip) if !IpNetwork::from(ip).is_global() => false,
+					multiaddr::Protocol::Ip6(ip) if !IpNetwork::from(ip).is_global() => false,
+					_ => true,
+				})
 			})
-		});
+			.collect::<Vec<_>>();
 
-		debug!(target: LOG_TARGET, "Authority DHT record peer_id='{:?}' addresses='{:?}'", peer_id, addresses.clone().collect::<Vec<_>>());
+		let peer_id = self.network.local_peer_id();
+		debug!(
+			target: LOG_TARGET,
+			"Authority DHT record peer_id='{peer_id}' addresses='{addresses:?}'",
+		);
 
-		// The address must include the peer id if not already set.
-		addresses.map(move |a| {
-			if a.iter().any(|p| matches!(p, multiaddr::Protocol::P2p(_))) {
-				a
-			} else {
-				a.with(multiaddr::Protocol::P2p(peer_id))
-			}
-		})
+		// The address must include the peer id.
+		let peer_id: Multihash = peer_id.into();
+		addresses.into_iter().map(move |a| a.with(multiaddr::Protocol::P2p(peer_id)))
 	}
 
 	/// Publish own public addresses.