diff --git a/substrate/client/authority-discovery/src/lib.rs b/substrate/client/authority-discovery/src/lib.rs index de98e6a4a38ae2c2aad7d9bdcb379c35c44287c2..e816600b7cde5e6a7970f46a6f4b4b4e1baf5ae1 100644 --- a/substrate/client/authority-discovery/src/lib.rs +++ b/substrate/client/authority-discovery/src/lib.rs @@ -58,19 +58,26 @@ use futures::task::{Context, Poll}; use futures::{Future, FutureExt, ready, Stream, StreamExt}; use futures_timer::Delay; +use addr_cache::AddrCache; use codec::Decode; use error::{Error, Result}; +use libp2p::core::multiaddr; use log::{debug, error, log_enabled}; use prometheus_endpoint::{Counter, CounterVec, Gauge, Opts, U64, register}; use prost::Message; use sc_client_api::blockchain::HeaderBackend; -use sc_network::{Multiaddr, config::MultiaddrWithPeerId, DhtEvent, ExHashT, NetworkStateInfo}; +use sc_network::{ + config::MultiaddrWithPeerId, + DhtEvent, + ExHashT, + Multiaddr, + NetworkStateInfo, +}; use sp_authority_discovery::{AuthorityDiscoveryApi, AuthorityId, AuthoritySignature, AuthorityPair}; use sp_core::crypto::{key_types, Pair}; use sp_core::traits::BareCryptoStorePtr; use sp_runtime::{traits::Block as BlockT, generic::BlockId}; use sp_api::ProvideRuntimeApi; -use addr_cache::AddrCache; #[cfg(test)] mod tests; @@ -233,7 +240,7 @@ where .collect(), None => self.network.external_addresses() .into_iter() - .map(|a| a.with(libp2p::core::multiaddr::Protocol::P2p( + .map(|a| a.with(multiaddr::Protocol::P2p( self.network.local_peer_id().into(), ))) .map(|a| a.to_vec()) @@ -423,6 +430,8 @@ where .get(&remote_key) .ok_or(Error::MatchingHashedAuthorityIdWithAuthorityId)?; + let local_peer_id = multiaddr::Protocol::P2p(self.network.local_peer_id().into()); + let remote_addresses: Vec<Multiaddr> = values.into_iter() .map(|(_k, v)| { let schema::SignedAuthorityAddresses { signature, addresses } = @@ -447,7 +456,13 @@ where Ok(addresses) }) .collect::<Result<Vec<Vec<Multiaddr>>>>()? - .into_iter().flatten().collect(); + .into_iter() + .flatten() + // Ignore own addresses. + .filter(|addr| !addr.iter().any(|protocol| + protocol == local_peer_id + )) + .collect(); if !remote_addresses.is_empty() { self.addr_cache.insert(authority_id.clone(), remote_addresses); diff --git a/substrate/client/authority-discovery/src/tests.rs b/substrate/client/authority-discovery/src/tests.rs index 12edcf5fc9008eb56dbd62aa3f637011de67bd94..09a65fd138c11ad7178fe91237a644e1d6e71a1b 100644 --- a/substrate/client/authority-discovery/src/tests.rs +++ b/substrate/client/authority-discovery/src/tests.rs @@ -24,10 +24,10 @@ use futures::future::{poll_fn, FutureExt}; use futures::sink::SinkExt; use futures::task::LocalSpawn; use futures::poll; -use libp2p::{kad, PeerId}; +use libp2p::{kad, core::multiaddr, PeerId}; use sp_api::{ProvideRuntimeApi, ApiRef}; -use sp_core::testing::KeyStore; +use sp_core::{crypto::Public, testing::KeyStore}; use sp_runtime::traits::{Zero, Block as BlockT, NumberFor}; use substrate_test_runtime_client::runtime::Block; @@ -210,7 +210,7 @@ impl NetworkStateInfo for TestNetwork { } fn external_addresses(&self) -> Vec<Multiaddr> { - vec!["/ip6/2001:db8::".parse().unwrap()] + vec!["/ip6/2001:db8::/tcp/30333".parse().unwrap()] } } @@ -281,7 +281,7 @@ fn publish_discover_cycle() { let peer_id = network.local_peer_id(); let address = network.external_addresses().pop().unwrap(); - address.with(libp2p::core::multiaddr::Protocol::P2p( + address.with(multiaddr::Protocol::P2p( peer_id.into(), )) }; @@ -461,3 +461,102 @@ fn dont_stop_polling_when_error_is_returned() { } ); } + +/// In the scenario of a validator publishing the address of its sentry node to +/// the DHT, said sentry node should not add its own Multiaddr to the +/// peerset "authority" priority group. +#[test] +fn never_add_own_address_to_priority_group() { + let validator_key_store = KeyStore::new(); + let validator_public = validator_key_store + .write() + .sr25519_generate_new(key_types::AUTHORITY_DISCOVERY, None) + .unwrap(); + + let sentry_network: Arc<TestNetwork> = Arc::new(Default::default()); + + let sentry_multiaddr = { + let peer_id = sentry_network.local_peer_id(); + let address: Multiaddr = "/ip6/2001:db8:0:0:0:0:0:2/tcp/30333".parse().unwrap(); + + address.with(multiaddr::Protocol::P2p( + peer_id.into(), + )) + }; + + // Address of some other sentry node of `validator`. + let random_multiaddr = { + let peer_id = PeerId::random(); + let address: Multiaddr = "/ip6/2001:db8:0:0:0:0:0:1/tcp/30333".parse().unwrap(); + + address.with(multiaddr::Protocol::P2p( + peer_id.into(), + )) + }; + + let dht_event = { + let addresses = vec![ + sentry_multiaddr.to_vec(), + random_multiaddr.to_vec(), + ]; + + let mut serialized_addresses = vec![]; + schema::AuthorityAddresses { addresses } + .encode(&mut serialized_addresses) + .map_err(Error::EncodingProto) + .unwrap(); + + let signature = validator_key_store.read() + .sign_with( + key_types::AUTHORITY_DISCOVERY, + &validator_public.clone().into(), + serialized_addresses.as_slice(), + ) + .map_err(|_| Error::Signing) + .unwrap(); + + let mut signed_addresses = vec![]; + schema::SignedAuthorityAddresses { + addresses: serialized_addresses.clone(), + signature, + } + .encode(&mut signed_addresses) + .map_err(Error::EncodingProto) + .unwrap(); + + let key = hash_authority_id(&validator_public.to_raw_vec()); + let value = signed_addresses; + (key, value) + }; + + let (_dht_event_tx, dht_event_rx) = channel(1); + let sentry_test_api = Arc::new(TestApi { + // Make sure the sentry node identifies its validator as an authority. + authorities: vec![validator_public.into()], + }); + + let mut sentry_authority_discovery = AuthorityDiscovery::new( + sentry_test_api, + sentry_network.clone(), + vec![], + dht_event_rx.boxed(), + Role::Sentry, + None, + ); + + sentry_authority_discovery.handle_dht_value_found_event(vec![dht_event]).unwrap(); + + assert_eq!( + sentry_network.set_priority_group_call.lock().unwrap().len(), 1, + "Expect authority discovery to set the priority set.", + ); + + assert_eq!( + sentry_network.set_priority_group_call.lock().unwrap()[0], + ( + "authorities".to_string(), + HashSet::from_iter(vec![random_multiaddr.clone()].into_iter(),) + ), + "Expect authority discovery to only add `random_multiaddr`." + ); +}