Authority-discovery no longer publishes non-global IP addresses (#8643)

* Authority-discovery no longer publishes non-global IP addresses

* Cargo.lock

* Update client/authority-discovery/src/lib.rs

Co-authored-by: Andronik Ordian <write@reusable.software>

Co-authored-by: Andronik Ordian <write@reusable.software>

substrate/Cargo.lock

@@ -6857,6 +6857,7 @@ dependencies = [
  "either",
  "futures 0.3.13",
  "futures-timer 3.0.2",
+ "ip_network",
  "libp2p",
  "log",
  "parity-scale-codec",

substrate/bin/node/cli/src/service.rs

@@ -218,6 +218,7 @@ pub fn new_full_base(
 	} = new_partial(&config)?;
 
 	let shared_voter_state = rpc_setup;
+	let auth_disc_publish_non_global_ips = config.network.allow_non_globals_in_dht;
 
 	config.network.extra_sets.push(grandpa::grandpa_peers_set_config());
 
@@ -320,7 +321,11 @@ pub fn new_full_base(
 				Event::Dht(e) => Some(e),
 				_ => None,
 			}});
-		let (authority_discovery_worker, _service) = sc_authority_discovery::new_worker_and_service(
+		let (authority_discovery_worker, _service) = sc_authority_discovery::new_worker_and_service_with_config(
+			sc_authority_discovery::WorkerConfig {
+				publish_non_global_ips: auth_disc_publish_non_global_ips,
+				..Default::default()
+			},
 			client.clone(),
 			network.clone(),
 			Box::pin(dht_event_stream),

substrate/client/authority-discovery/Cargo.toml

@@ -23,6 +23,7 @@ derive_more = "0.99.2"
 either = "1.5.3"
 futures = "0.3.9"
 futures-timer = "3.0.1"
+ip_network = "0.3.4"
 libp2p = { version = "0.37.1", default-features = false, features = ["kad"] }
 log = "0.4.8"
 prometheus-endpoint = { package = "substrate-prometheus-endpoint", path = "../../utils/prometheus", version = "0.9.0"}

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

@@ -62,6 +62,14 @@ pub struct WorkerConfig {
 	///
 	/// By default this is set to 10 minutes.
 	pub max_query_interval: Duration,
+
+	/// If `false`, the node won't publish on the DHT multiaddresses that contain non-global
+	/// IP addresses (such as 10.0.0.1).
+	///
+	/// Recommended: `false` for live chains, and `true` for local chains or for testing.
+	///
+	/// Defaults to `true` to avoid the surprise factor.
+	pub publish_non_global_ips: bool,
 }
 
 impl Default for WorkerConfig {
@@ -81,6 +89,7 @@ impl Default for WorkerConfig {
 			// comparing `authority_discovery_authority_addresses_requested_total` and
 			// `authority_discovery_dht_event_received`.
 			max_query_interval: Duration::from_secs(10 * 60),
+			publish_non_global_ips: true,
 		}
 	}
 }

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

@@ -30,6 +30,7 @@ use futures::{future, FutureExt, Stream, StreamExt, stream::Fuse};
 use addr_cache::AddrCache;
 use async_trait::async_trait;
 use codec::Decode;
+use ip_network::IpNetwork;
 use libp2p::{core::multiaddr, multihash::{Multihash, Hasher}};
 use log::{debug, error, log_enabled};
 use prometheus_endpoint::{Counter, CounterVec, Gauge, Opts, U64, register};
@@ -115,6 +116,8 @@ pub struct Worker<Client, Network, Block, DhtEventStream> {
 	/// List of keys onto which addresses have been published at the latest publication.
 	/// Used to check whether they have changed.
 	latest_published_keys: HashSet<CryptoTypePublicPair>,
+	/// Same value as in the configuration.
+	publish_non_global_ips: bool,
 
 	/// Interval at which to request addresses of authorities, refilling the pending lookups queue.
 	query_interval: ExpIncInterval,
@@ -197,6 +200,7 @@ where
 			publish_interval,
 			publish_if_changed_interval,
 			latest_published_keys: HashSet::new(),
+			publish_non_global_ips: config.publish_non_global_ips,
 			query_interval,
 			pending_lookups: Vec::new(),
 			in_flight_lookups: HashMap::new(),
@@ -267,10 +271,24 @@ where
 		}
 	}
 
-	fn addresses_to_publish(&self) -> impl ExactSizeIterator<Item = Multiaddr> {
+	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;
 		self.network.external_addresses()
 			.into_iter()
+			.filter(move |a| {
+				if publish_non_global_ips {
+					return true;
+				}
+
+				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,
+				})
+			})
 			.map(move |a| {
 				if a.iter().any(|p| matches!(p, multiaddr::Protocol::P2p(_))) {
 					a
@@ -299,7 +317,7 @@ where
 			return Ok(())
 		}
 
-		let addresses = self.addresses_to_publish();
+		let addresses = self.addresses_to_publish().map(|a| a.to_vec()).collect::<Vec<_>>();
 
 		if let Some(metrics) = &self.metrics {
 			metrics.publish.inc();
@@ -309,7 +327,7 @@ where
 		}
 
 		let mut serialized_addresses = vec![];
-		schema::AuthorityAddresses { addresses: addresses.map(|a| a.to_vec()).collect() }
+		schema::AuthorityAddresses { addresses }
 			.encode(&mut serialized_addresses)
 			.map_err(Error::EncodingProto)?;