client/authority-discovery: Allow to be run by sentry node

.maintain/sentry-node/docker-compose.yml

@@ -79,7 +79,6 @@ services:
       - "--chain=local"
       - "--port"
       - "30333"
-      - "--charlie"
       - "--bootnodes"
       - "/dns4/validator-a/tcp/30333/p2p/QmRpheLN4JWdAnY7HGJfWFNbfkQCb6tFf4vvA6hgjMZKrR"
       - "--bootnodes"
@@ -95,6 +94,7 @@ services:
       - "--log"
       - "sub-authority-discovery=trace"
       - "--sentry"
+      - "/dns4/validator-a/tcp/30333/p2p/QmRpheLN4JWdAnY7HGJfWFNbfkQCb6tFf4vvA6hgjMZKrR"
 
   validator-b:
     image: parity/substrate

bin/node/cli/src/service.rs

@@ -146,7 +146,7 @@ macro_rules! new_full {
 
 		($with_startup_data)(&block_import, &babe_link);
 
-		if let sc_service::config::Role::Authority { sentry_nodes } = &role {
+		if let sc_service::config::Role::Authority { .. } = &role {
 			let proposer = sc_basic_authorship::ProposerFactory::new(
 				service.client(),
 				service.transaction_pool()
@@ -174,6 +174,26 @@ macro_rules! new_full {
 
 			let babe = sc_consensus_babe::start_babe(babe_config)?;
 			service.spawn_essential_task("babe-proposer", babe);
+		}
+
+		// Spawn authority discovery module.
+		if matches!(role, sc_service::config::Role::Authority{..} | sc_service::config::Role::Sentry{..}) {
+			let sentries;
+			let authority_discovery_role;
+
+			match role {
+				sc_service::config::Role::Authority { ref sentry_nodes } => {
+					sentries = sentry_nodes.clone();
+					authority_discovery_role = sc_authority_discovery::Role::Authority (
+						service.keystore(),
+					);
+				}
+				sc_service::config::Role::Sentry {..} => {
+					sentries = vec![];
+					authority_discovery_role = sc_authority_discovery::Role::Sentry;
+				}
+				_ => unreachable!("Due to outer matches! constraint; qed.")
+			}
 
 			let network = service.network();
 			let dht_event_stream = network.event_stream().filter_map(|e| async move { match e {
@@ -183,9 +203,9 @@ macro_rules! new_full {
 			let authority_discovery = sc_authority_discovery::AuthorityDiscovery::new(
 				service.client(),
 				network,
-				sentry_nodes.clone(),
-				service.keystore(),
+				sentries,
 				dht_event_stream,
+				authority_discovery_role,
 				service.prometheus_registry(),
 			);
 

client/authority-discovery/src/lib.rs

@@ -25,13 +25,11 @@
 //!
 //! 1. **Makes itself discoverable**
 //!
-//!    1. Retrieves its external addresses.
+//!    1. Retrieves its external addresses (including peer id) or the ones of its sentry nodes.
 //!
-//!    2. Adds its network peer id to the addresses.
+//!    2. Signs the above.
 //!
-//!    3. Signs the above.
-//!
-//!    4. Puts the signature and the addresses on the libp2p Kademlia DHT.
+//!    3. Puts the signature and the addresses on the libp2p Kademlia DHT.
 //!
 //!
 //! 2. **Discovers other authorities**
@@ -43,6 +41,10 @@
 //!    3. Validates the signatures of the retrieved key value pairs.
 //!
 //!    4. Adds the retrieved external addresses as priority nodes to the peerset.
+//!
+//!
+//! When run as a sentry node step 1 is skipped and only step 2 is executed.
+
 use std::collections::{HashMap, HashSet};
 use std::convert::TryInto;
 use std::marker::PhantomData;
@@ -66,6 +68,7 @@ use sp_core::crypto::{key_types, CryptoTypePublicPair, Pair};
 use sp_core::traits::BareCryptoStorePtr;
 use sp_runtime::{traits::Block as BlockT, generic::BlockId};
 use sp_api::ProvideRuntimeApi;
+// TODO: Needed?
 use addr_cache::AddrCache;
 
 #[cfg(test)]
@@ -109,7 +112,8 @@ impl Metrics {
 			amount_last_published: register(
 				Gauge::new(
 					"authority_discovery_amount_external_addresses_last_published",
-					"Number of external addresses published when authority discovery last published addresses ."
+					"Number of external addresses published when authority discovery last \
+					 published addresses ."
 				)?,
 				registry,
 			)?,
@@ -134,6 +138,17 @@ impl Metrics {
 	}
 }
 
+/// Role an authority discovery module can run as.
+pub enum Role {
+	/// Actual authority as well as a reference to its key store.
+	Authority(BareCryptoStorePtr),
+	/// Sentry node that guards an authority.
+	///
+	/// No reference to its key store needed, as sentry nodes don't have an identity to sign
+	/// addresses with in the first place.
+	Sentry,
+}
+
 /// An `AuthorityDiscovery` makes a given authority discoverable and discovers other authorities.
 pub struct AuthorityDiscovery<Client, Network, Block>
 where
@@ -156,8 +171,6 @@ where
 	/// Channel we receive Dht events on.
 	dht_event_rx: Pin<Box<dyn Stream<Item = DhtEvent> + Send>>,
 
-	key_store: BareCryptoStorePtr,
-
 	/// Interval to be proactive, publishing own addresses.
 	publish_interval: Interval,
 	/// Interval on which to query for addresses of other authorities.
@@ -167,6 +180,8 @@ where
 
 	metrics: Option<Metrics>,
 
+	role: Role,
+
 	phantom: PhantomData<Block>,
 }
 
@@ -187,8 +202,8 @@ where
 		client: Arc<Client>,
 		network: Arc<Network>,
 		sentry_nodes: Vec<MultiaddrWithPeerId>,
-		key_store: BareCryptoStorePtr,
 		dht_event_rx: Pin<Box<dyn Stream<Item = DhtEvent> + Send>>,
+		role: Role,
 		prometheus_registry: Option<prometheus_endpoint::Registry>,
 	) -> Self {
 		// Kademlia's default time-to-live for Dht records is 36h, republishing records every 24h.
@@ -221,7 +236,10 @@ where
 				match Metrics::register(&registry) {
 					Ok(metrics) => Some(metrics),
 					Err(e) => {
-						error!(target: "sub-authority-discovery", "Failed to register metrics: {:?}", e);
+						error!(
+							target: "sub-authority-discovery",
+							"Failed to register metrics: {:?}", e,
+						);
 						None
 					},
 				}
@@ -234,17 +252,25 @@ where
 			network,
 			sentry_nodes,
 			dht_event_rx,
-			key_store,
 			publish_interval,
 			query_interval,
 			addr_cache,
+			role,
 			metrics,
 			phantom: PhantomData,
 		}
 	}
 
 	/// Publish either our own or if specified the public addresses of our sentry nodes.
 	fn publish_ext_addresses(&mut self) -> Result<()> {
+		let key_store = match &self.role {
+			Role::Authority(key_store) => key_store,
+			// Only authority nodes can put addresses (their own or the ones of their sentry nodes)
+			// on the Dht. Sentry nodes don't have a known identity to authenticate such addresses,
+			// thus `publish_ext_addresses` becomes a no-op.
+			Role::Sentry => return Ok(()),
+		};
+
 		if let Some(metrics) = &self.metrics {
 			metrics.publish.inc()
 		}
@@ -271,13 +297,12 @@ where
 			.encode(&mut serialized_addresses)
 			.map_err(Error::EncodingProto)?;
 
-		let keys: Vec<CryptoTypePublicPair> = self.get_own_public_keys_within_authority_set()?
-			.into_iter()
-			.map(Into::into)
-			.collect();
+		let keys = AuthorityDiscovery::get_own_public_keys_within_authority_set(
+			&key_store,
+			&self.client,
+		)?.into_iter().map(Into::into).collect::<Vec<_> >();
 
-		let signatures = self.key_store
-			.read()
+		let signatures = key_store.read()
 			.sign_with_all(
 				key_types::AUTHORITY_DISCOVERY,
 				keys.clone(),
@@ -457,17 +482,17 @@ where
 	// one for the upcoming session. In addition it could be participating in the current authority
 	// set with two keys. The function does not return all of the local authority discovery public
 	// keys, but only the ones intersecting with the current authority set.
-	fn get_own_public_keys_within_authority_set(&mut self) -> Result<HashSet<AuthorityId>> {
-		let local_pub_keys = self.key_store
-			.read()
+	fn get_own_public_keys_within_authority_set(
+		key_store: &BareCryptoStorePtr,
+		client: &Client,
+	) -> Result<HashSet<AuthorityId>> {
+		let local_pub_keys = key_store.read()
 			.sr25519_public_keys(key_types::AUTHORITY_DISCOVERY)
 			.into_iter()
 			.collect::<HashSet<_>>();
 
-		let id = BlockId::hash(self.client.info().best_hash);
-		let current_authorities = self
-			.client
-			.runtime_api()
+		let id = BlockId::hash(client.info().best_hash);
+		let current_authorities = client.runtime_api()
 			.authorities(&id)
 			.map_err(Error::CallingRuntime)?
 			.into_iter()
@@ -492,7 +517,10 @@ where
 			"Applying priority group {:?} to peerset.", addresses,
 		);
 		self.network
-			.set_priority_group(AUTHORITIES_PRIORITY_GROUP_NAME.to_string(), addresses.into_iter().collect())
+			.set_priority_group(
+				AUTHORITIES_PRIORITY_GROUP_NAME.to_string(),
+				addresses.into_iter().collect(),
+			)
 			.map_err(Error::SettingPeersetPriorityGroup)?;
 
 		Ok(())

client/authority-discovery/src/tests.rs

@@ -213,8 +213,8 @@ fn new_registers_metrics() {
 		test_api,
 		network.clone(),
 		vec![],
-		key_store,
 		dht_event_rx.boxed(),
+		Role::Authority(key_store),
 		Some(registry.clone()),
 	);
 
@@ -238,8 +238,8 @@ fn publish_ext_addresses_puts_record_on_dht() {
 		test_api,
 		network.clone(),
 		vec![],
-		key_store,
 		dht_event_rx.boxed(),
+		Role::Authority(key_store),
 		None,
 	);
 
@@ -269,8 +269,8 @@ fn request_addresses_of_others_triggers_dht_get_query() {
 		test_api,
 		network.clone(),
 		vec![],
-		key_store,
 		dht_event_rx.boxed(),
+		Role::Authority(key_store),
 		None,
 	);
 
@@ -297,8 +297,8 @@ fn handle_dht_events_with_value_found_should_call_set_priority_group() {
 		test_api,
 		network.clone(),
 		vec![],
-		key_store,
 		dht_event_rx.boxed(),
+		Role::Authority(key_store),
 		None,
 	);