Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 6 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
9 changes: 6 additions & 3 deletions client/authority-discovery/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#![warn(missing_docs)]
#![recursion_limit = "1024"]

//! Substrate authority discovery.
//!
//! This crate enables Substrate authorities to discover and directly connect to
Expand All @@ -31,7 +32,7 @@ pub use crate::{
worker::{NetworkProvider, Role, Worker},
};

use std::{sync::Arc, time::Duration};
use std::{collections::HashSet, sync::Arc, time::Duration};

use futures::{
channel::{mpsc, oneshot},
Expand All @@ -58,11 +59,13 @@ pub struct WorkerConfig {
///
/// By default this is set to 1 hour.
pub max_publish_interval: Duration,

/// Interval at which the keystore is queried. If the keys have changed, unconditionally
/// re-publish its addresses on the DHT.
///
/// By default this is set to 1 minute.
pub keystore_refresh_interval: Duration,

/// The maximum interval in which the node will query the DHT for new entries.
///
/// By default this is set to 10 minutes.
Expand Down Expand Up @@ -156,7 +159,7 @@ where
/// Message send from the [`Service`] to the [`Worker`].
pub(crate) enum ServicetoWorkerMsg {
/// See [`Service::get_addresses_by_authority_id`].
GetAddressesByAuthorityId(AuthorityId, oneshot::Sender<Option<Vec<Multiaddr>>>),
GetAddressesByAuthorityId(AuthorityId, oneshot::Sender<Option<HashSet<Multiaddr>>>),
/// See [`Service::get_authority_id_by_peer_id`].
GetAuthorityIdByPeerId(PeerId, oneshot::Sender<Option<AuthorityId>>),
GetAuthorityIdByPeerId(PeerId, oneshot::Sender<Option<HashSet<AuthorityId>>>),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the Option redundant here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends on how you see it. None tells you that the peer id is unknown.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would also know that, if the set is empty - no? Like if there is an entry for the PeerId the set should not be empty, otherwise, why would we keep the entry?

Comment thread
ordian marked this conversation as resolved.
Outdated
}
9 changes: 6 additions & 3 deletions client/authority-discovery/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use std::fmt::Debug;
use std::{collections::HashSet, fmt::Debug};

use crate::ServicetoWorkerMsg;

Expand Down Expand Up @@ -62,7 +62,7 @@ impl Service {
pub async fn get_addresses_by_authority_id(
&mut self,
authority: AuthorityId,
) -> Option<Vec<Multiaddr>> {
) -> Option<HashSet<Multiaddr>> {
let (tx, rx) = oneshot::channel();

self.to_worker
Expand All @@ -78,7 +78,10 @@ impl Service {
///
/// Returns `None` if no entry was present or connection to the
/// [`crate::Worker`] failed.
pub async fn get_authority_id_by_peer_id(&mut self, peer_id: PeerId) -> Option<AuthorityId> {
pub async fn get_authority_id_by_peer_id(
Comment thread
ordian marked this conversation as resolved.
Outdated
&mut self,
peer_id: PeerId,
) -> Option<HashSet<AuthorityId>> {
let (tx, rx) = oneshot::channel();

self.to_worker
Expand Down
6 changes: 3 additions & 3 deletions client/authority-discovery/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use libp2p::core::{
multiaddr::{Multiaddr, Protocol},
PeerId,
};
use std::sync::Arc;
use std::{collections::HashSet, sync::Arc};

use sp_authority_discovery::AuthorityId;
use sp_core::crypto::key_types;
Expand Down Expand Up @@ -73,11 +73,11 @@ fn get_addresses_and_authority_id() {

pool.run_until(async {
assert_eq!(
Some(vec![remote_addr]),
Some(HashSet::from([remote_addr])),
service.get_addresses_by_authority_id(remote_authority_id.clone()).await,
);
assert_eq!(
Some(remote_authority_id),
Some(HashSet::from([remote_authority_id])),
service.get_authority_id_by_peer_id(remote_peer_id).await,
);
});
Expand Down
6 changes: 3 additions & 3 deletions client/authority-discovery/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ where
},
ServicetoWorkerMsg::GetAuthorityIdByPeerId(peer_id, sender) => {
let _ = sender
.send(self.addr_cache.get_authority_id_by_peer_id(&peer_id).map(Clone::clone));
.send(self.addr_cache.get_authority_ids_by_peer_id(&peer_id).map(Clone::clone));
},
}
}
Expand Down Expand Up @@ -374,7 +374,7 @@ where
.map_err(|e| Error::CallingRuntime(e.into()))?
.into_iter()
.filter(|id| !local_keys.contains(id.as_ref()))
.collect();
.collect::<Vec<_>>();

self.addr_cache.retain_ids(&authorities);

Expand Down Expand Up @@ -548,7 +548,7 @@ where
if let Some(metrics) = &self.metrics {
metrics
.known_authorities_count
.set(self.addr_cache.num_ids().try_into().unwrap_or(std::u64::MAX));
.set(self.addr_cache.num_authority_ids().try_into().unwrap_or(std::u64::MAX));
}
}
Ok(())
Expand Down
Loading