Skip to content

Commit

Permalink
feat(kademlia)!: use GATs on RecordStore trait (#3239)
Browse files Browse the repository at this point in the history
Previously, we applied a lifetime onto the entire `RecordStore` to workaround Rust not having GATs. With Rust 1.65.0 we now have GATs so we can remove this workaround.

Related #3240. Without this change, we would have to specify HRTB in various places.
  • Loading branch information
thomaseizinger authored Dec 22, 2022
1 parent 65ec545 commit 1765ae0
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 31 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@
- Remove `BandwidthFuture`
- Rename `BandwidthConnecLogging` to `InstrumentedStream`
- Remove `SimpleProtocol` due to being unused. See [`libp2p::core::upgrade`](https://docs.rs/libp2p/0.50.0/libp2p/core/upgrade/index.html) for alternatives. See [PR 3191].

- Bump MSRV to 1.65.0.

- Update individual crates.
- Update to [`libp2p-dcutr` `v0.9.0`](protocols/dcutr/CHANGELOG.md#090).

Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "libp2p"
edition = "2021"
rust-version = "1.62.0"
rust-version = "1.65.0"
description = "Peer-to-peer networking library"
version = "0.51.0"
authors = ["Parity Technologies <[email protected]>"]
Expand Down
2 changes: 2 additions & 0 deletions misc/metrics/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

- Add `connections_establishment_duration` metric. See [PR 3134].

- Bump MSRV to 1.65.0.

- Update to `libp2p-dcutr` `v0.9.0`.

- Update to `libp2p-ping` `v0.42.0`.
Expand Down
2 changes: 1 addition & 1 deletion misc/metrics/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "libp2p-metrics"
edition = "2021"
rust-version = "1.62.0"
rust-version = "1.65.0"
description = "Metrics for libp2p"
version = "0.12.0"
authors = ["Max Inden <[email protected]>"]
Expand Down
6 changes: 6 additions & 0 deletions protocols/kad/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

- Update to `libp2p-swarm` `v0.42.0`.

- Remove lifetime from `RecordStore` and use GATs instead. See [PR 3239].

- Bump MSRV to 1.65.0.

[PR 3239]: https://github.com/libp2p/rust-libp2p/pull/3239

# 0.42.0

- Update to `libp2p-core` `v0.38.0`.
Expand Down
2 changes: 1 addition & 1 deletion protocols/kad/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "libp2p-kad"
edition = "2021"
rust-version = "1.62.0"
rust-version = "1.65.0"
description = "Kademlia protocol for libp2p"
version = "0.43.0"
authors = ["Parity Technologies <[email protected]>"]
Expand Down
6 changes: 2 additions & 4 deletions protocols/kad/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,8 +401,7 @@ impl KademliaConfig {

impl<TStore> Kademlia<TStore>
where
for<'a> TStore: RecordStore<'a>,
TStore: Send + 'static,
TStore: RecordStore + Send + 'static,
{
/// Creates a new `Kademlia` network behaviour with a default configuration.
pub fn new(id: PeerId, store: TStore) -> Self {
Expand Down Expand Up @@ -1987,8 +1986,7 @@ fn exp_decrease(ttl: Duration, exp: u32) -> Duration {

impl<TStore> NetworkBehaviour for Kademlia<TStore>
where
for<'a> TStore: RecordStore<'a>,
TStore: Send + 'static,
TStore: RecordStore + Send + 'static,
{
type ConnectionHandler = KademliaHandlerProto<QueryId>;
type OutEvent = KademliaEvent;
Expand Down
4 changes: 2 additions & 2 deletions protocols/kad/src/jobs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ impl PutRecordJob {
/// to be run.
pub fn poll<T>(&mut self, cx: &mut Context<'_>, store: &mut T, now: Instant) -> Poll<Record>
where
for<'a> T: RecordStore<'a>,
T: RecordStore,
{
if self.inner.check_ready(cx, now) {
let publish = self.next_publish.map_or(false, |t_pub| now >= t_pub);
Expand Down Expand Up @@ -294,7 +294,7 @@ impl AddProviderJob {
now: Instant,
) -> Poll<ProviderRecord>
where
for<'a> T: RecordStore<'a>,
T: RecordStore,
{
if self.inner.check_ready(cx, now) {
let records = store
Expand Down
26 changes: 15 additions & 11 deletions protocols/kad/src/record/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,36 +64,40 @@ pub enum Error {
/// content. Just like a regular record, a provider record is distributed
/// to the closest nodes to the key.
///
pub trait RecordStore<'a> {
type RecordsIter: Iterator<Item = Cow<'a, Record>>;
type ProvidedIter: Iterator<Item = Cow<'a, ProviderRecord>>;
pub trait RecordStore {
type RecordsIter<'a>: Iterator<Item = Cow<'a, Record>>
where
Self: 'a;
type ProvidedIter<'a>: Iterator<Item = Cow<'a, ProviderRecord>>
where
Self: 'a;

/// Gets a record from the store, given its key.
fn get(&'a self, k: &Key) -> Option<Cow<'_, Record>>;
fn get(&self, k: &Key) -> Option<Cow<'_, Record>>;

/// Puts a record into the store.
fn put(&'a mut self, r: Record) -> Result<()>;
fn put(&mut self, r: Record) -> Result<()>;

/// Removes the record with the given key from the store.
fn remove(&'a mut self, k: &Key);
fn remove(&mut self, k: &Key);

/// Gets an iterator over all (value-) records currently stored.
fn records(&'a self) -> Self::RecordsIter;
fn records(&self) -> Self::RecordsIter<'_>;

/// Adds a provider record to the store.
///
/// A record store only needs to store a number of provider records
/// for a key corresponding to the replication factor and should
/// store those records whose providers are closest to the key.
fn add_provider(&'a mut self, record: ProviderRecord) -> Result<()>;
fn add_provider(&mut self, record: ProviderRecord) -> Result<()>;

/// Gets a copy of the stored provider records for the given key.
fn providers(&'a self, key: &Key) -> Vec<ProviderRecord>;
fn providers(&self, key: &Key) -> Vec<ProviderRecord>;

/// Gets an iterator over all stored provider records for which the
/// node owning the store is itself the provider.
fn provided(&'a self) -> Self::ProvidedIter;
fn provided(&self) -> Self::ProvidedIter<'_>;

/// Removes a provider record from the store.
fn remove_provider(&'a mut self, k: &Key, p: &PeerId);
fn remove_provider(&mut self, k: &Key, p: &PeerId);
}
22 changes: 11 additions & 11 deletions protocols/kad/src/record/store/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,20 +96,20 @@ impl MemoryStore {
}
}

impl<'a> RecordStore<'a> for MemoryStore {
type RecordsIter =
impl RecordStore for MemoryStore {
type RecordsIter<'a> =
iter::Map<hash_map::Values<'a, Key, Record>, fn(&'a Record) -> Cow<'a, Record>>;

type ProvidedIter = iter::Map<
type ProvidedIter<'a> = iter::Map<
hash_set::Iter<'a, ProviderRecord>,
fn(&'a ProviderRecord) -> Cow<'a, ProviderRecord>,
>;

fn get(&'a self, k: &Key) -> Option<Cow<'_, Record>> {
fn get(&self, k: &Key) -> Option<Cow<'_, Record>> {
self.records.get(k).map(Cow::Borrowed)
}

fn put(&'a mut self, r: Record) -> Result<()> {
fn put(&mut self, r: Record) -> Result<()> {
if r.value.len() >= self.config.max_value_bytes {
return Err(Error::ValueTooLarge);
}
Expand All @@ -131,15 +131,15 @@ impl<'a> RecordStore<'a> for MemoryStore {
Ok(())
}

fn remove(&'a mut self, k: &Key) {
fn remove(&mut self, k: &Key) {
self.records.remove(k);
}

fn records(&'a self) -> Self::RecordsIter {
fn records(&self) -> Self::RecordsIter<'_> {
self.records.values().map(Cow::Borrowed)
}

fn add_provider(&'a mut self, record: ProviderRecord) -> Result<()> {
fn add_provider(&mut self, record: ProviderRecord) -> Result<()> {
let num_keys = self.providers.len();

// Obtain the entry
Expand Down Expand Up @@ -189,17 +189,17 @@ impl<'a> RecordStore<'a> for MemoryStore {
Ok(())
}

fn providers(&'a self, key: &Key) -> Vec<ProviderRecord> {
fn providers(&self, key: &Key) -> Vec<ProviderRecord> {
self.providers
.get(key)
.map_or_else(Vec::new, |ps| ps.clone().into_vec())
}

fn provided(&'a self) -> Self::ProvidedIter {
fn provided(&self) -> Self::ProvidedIter<'_> {
self.provided.iter().map(Cow::Borrowed)
}

fn remove_provider(&'a mut self, key: &Key, provider: &PeerId) {
fn remove_provider(&mut self, key: &Key, provider: &PeerId) {
if let hash_map::Entry::Occupied(mut e) = self.providers.entry(key.clone()) {
let providers = e.get_mut();
if let Some(i) = providers.iter().position(|p| &p.provider == provider) {
Expand Down

0 comments on commit 1765ae0

Please sign in to comment.