Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

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

Merged
3 commits merged into from
Apr 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions client/authority-discovery/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down
9 changes: 9 additions & 0 deletions client/authority-discovery/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
}
}
}
Expand Down
24 changes: 21 additions & 3 deletions client/authority-discovery/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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)?;

Expand Down